🐞 Director correctly places runtime config jobs... #2475
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
...when the deployment manifest does not list the stemcell OS. Manifests can select stemcells by name or OS (but not both). <https://bosh.io/docs/manifest-v2/#stemcells> Runtime config jobs can be placed by stemcell OS. <https://bosh.io/docs/runtime-config/#placement-rules> If your manifest uses stemcell name, even if that stemcell has an OS that matches the runtime config, it will not be used. Stemcells in manifest: ``` stemcells: - alias: default name: bosh-aws-xen-hvm-ubuntu-jammy-go_agent version: latest ``` Placement rules in runtime config: ``` include: stemcell: - os: ubuntu-jammy ``` This commit fixes that shortcoming; now if the manifest specifies stemcells by name, the Director will appropriately place the runtime config jobs on the VMs. Signed-off-by: Brian Cunnie <bcunnie@vmware.com>
jpalermo
requested review from
a team,
beyhan and
selzoc
and removed request for
a team
beyhan
approved these changes
Nov 10, 2023
rkoster
approved these changes
Nov 16, 2023
jpalermo
pushed a commit
that referenced
this pull request
Jan 3, 2024
Follow up to: #2475 The previous PR didn't handle diffs correctly so it showed that runtime configs were getting added/removed even though they weren't. This commit moves the OS population up into the manifest parsing so it can apply to both the deployment manifest and the diff manifest. This caused the DeploymentPlan Stemcell to start failing because it had a validation that both Name and OS shouldn't be specified. We moved that validation up to the ManifestValidator which seems like a more appropriate place for it anyway. We modified the manifest parsing to prefer Name over OS when resolving the stemcell version aliases. This is more consistent with how the DeploymentPlan Stemcell resolves Name and OS. It shouldn't be a behavioral change though since you can't specify both Name and OS. Signed-off-by: Joseph Palermo <joseph.palermo@broadcom.com>
jpalermo
pushed a commit
that referenced
this pull request
Jan 3, 2024
Follow up to: #2475 The previous PR didn't handle diffs correctly so it showed that runtime configs were getting added/removed even though they weren't. This commit moves the OS population up into the manifest parsing so it can apply to both the deployment manifest and the diff manifest. This caused the DeploymentPlan Stemcell to start failing because it had a validation that both Name and OS shouldn't be specified. We moved that validation up to the ManifestValidator which seems like a more appropriate place for it anyway. We modified the manifest parsing to prefer Name over OS when resolving the stemcell version aliases. This is more consistent with how the DeploymentPlan Stemcell resolves Name and OS. It shouldn't be a behavioral change though since you can't specify both Name and OS. Signed-off-by: Joseph Palermo <joseph.palermo@broadcom.com>
jpalermo
pushed a commit
that referenced
this pull request
Jan 8, 2024
Follow up to: #2475 The previous PR didn't handle diffs correctly so it showed that runtime configs were getting added/removed even though they weren't. This commit moves the OS population up into the manifest parsing so it can apply to both the deployment manifest and the diff manifest. This caused the DeploymentPlan Stemcell to start failing because it had a validation that both Name and OS shouldn't be specified. We moved that validation up to the ManifestValidator which seems like a more appropriate place for it anyway. We modified the manifest parsing to prefer Name over OS when resolving the stemcell version aliases. This is more consistent with how the DeploymentPlan Stemcell resolves Name and OS. It shouldn't be a behavioral change though since you can't specify both Name and OS. Signed-off-by: Joseph Palermo <joseph.palermo@broadcom.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
...when the deployment manifest does not list the stemcell OS.
Manifests can select stemcells by name or OS (but not both). https://bosh.io/docs/manifest-v2/#stemcells
Runtime config jobs can be placed by stemcell OS. https://bosh.io/docs/runtime-config/#placement-rules
If your manifest uses stemcell name, even if that stemcell has an OS that matches the runtime config, it will not be used. Stemcells in manifest:
Placement rules in runtime config:
This commit fixes that shortcoming; now if the manifest specifies stemcells by name, the Director will appropriately place the runtime config jobs on the VMs.