Skip to content

reorder volume and volume_mounts parameter#168

Open
remenaker wants to merge 1 commit intorundeck-plugins:masterfrom
remenaker:fix-parameter-order
Open

reorder volume and volume_mounts parameter#168
remenaker wants to merge 1 commit intorundeck-plugins:masterfrom
remenaker:fix-parameter-order

Conversation

@remenaker
Copy link
Copy Markdown

This PR is to fix the order of the Volume and Volume Mounts parameters. We had an issue in were options were copied from the Pod/Create workflow step to the Job/Create workflow step. When copying the options from one to another it went un-noticed that the volumes and volume_mounts were switched between the steps, thus volumes were not created correctly. It took some time to notice this and correct it. Hopefully this will save some others headache in the future. Thanks!

@fdevans
Copy link
Copy Markdown
Contributor

fdevans commented Mar 6, 2026

Thank you for identifying this inconsistency! You're absolutely right that having different parameter ordering between Pod/Create and Job/Create steps causes confusion when copying configurations.

However, we'd like to suggest the opposite fix. Your PR makes Job/Create match Pod/Create by putting volume_mounts before volumes, but this is actually backwards from a logical standpoint.

In Kubernetes:

  1. You define volumes first (what storage to make available)
  2. Then you define volume_mounts (how to mount those volumes into containers)

Current state:

  • Pod/Create: volume_mountsvolumes (backwards)
  • Job/Create: volumesvolume_mounts (correct logical order)

Recommendation:
Instead of this PR, we should fix Pod/Create to match Job/Create's ordering. This would:

  • Maintain consistency across steps (your goal)
  • Follow the logical Kubernetes order (volumes before mounts)
  • Match how users think about the configuration

Would you be willing to update this PR to fix Pod/Create instead, making it match Job/Create's ordering? That would put volumes before volume_mounts in both places.

Also, please rebase on the latest master branch before updating.

Thanks for catching this inconsistency!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants