Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Get rid of envsubst for nameprefix #1126

Merged
merged 1 commit into from Jun 17, 2022

Conversation

lentzi90
Copy link
Member

@lentzi90 lentzi90 commented May 31, 2022

The Ironic kustomizations used envsubst to set the nameprefix, while BMO
was using the built in feature in kustomize. To unify them and simplify
the deploy script a bit, the Ironix kustomizations were changed to also
use the kustomize feature.

There is a slight change in the manifests since the namePrefix is
applied also to secrets, configmaps and label selectors that didn't have
the prefix set before.

The namespace is now also created as part of the kustomizations for
Ironic (same as for BMO).

Fixes #1072

Edit:

This breaks CAPM3 v1alpha4 (which is deprecated). The issue is that Ironic ends up in the wrong namespace. It is easily solve by copying the ironic-cacert secret to the namespace where BMO is running (capm3-system) or making changes to the Ironic manifests. However, v1alpha4 is no longer supported so I'm a bit hesitant to add something specific to that version.

In addition to this, v1alpha4 uses Kubernetes v1.21, which is EOL in just a few weeks. So it makes sense to not focus on this old version in my opinion. But of course, let me know if there are objections!

@metal3-io-bot
Copy link
Contributor

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@metal3-io-bot metal3-io-bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels May 31, 2022
@lentzi90
Copy link
Member Author

/test-v1b1-ubuntu-integration

@lentzi90 lentzi90 force-pushed the lentzi90/no-envsubst-prefix branch from 556ec71 to 5a874b8 Compare June 1, 2022 08:43
@lentzi90
Copy link
Member Author

lentzi90 commented Jun 1, 2022

/test-v1b1-ubuntu-integration

@lentzi90 lentzi90 force-pushed the lentzi90/no-envsubst-prefix branch from 5a874b8 to 0e2c464 Compare June 2, 2022 11:33
The Ironic kustomizations used envsubst to set the nameprefix, while BMO
was using the built in feature in kustomize. To unify them and simplify
the deploy script a bit, the Ironix kustomizations were changed to also
use the kustomize feature.

There is a slight change in the manifests since the namePrefix is
applied also to secrets, configmaps and label selectors that didn't have
the prefix set before.

The namespace is now also created as part of the kustomizations for
Ironic (same as for BMO).
@lentzi90 lentzi90 force-pushed the lentzi90/no-envsubst-prefix branch from 0e2c464 to c96d939 Compare June 2, 2022 11:41
@lentzi90 lentzi90 marked this pull request as ready for review June 2, 2022 11:43
@metal3-io-bot metal3-io-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 2, 2022
@lentzi90
Copy link
Member Author

lentzi90 commented Jun 3, 2022

/test-v1b1-ubuntu-integration

1 similar comment
@lentzi90
Copy link
Member Author

lentzi90 commented Jun 3, 2022

/test-v1b1-ubuntu-integration

@lentzi90
Copy link
Member Author

lentzi90 commented Jun 3, 2022

/test-v1b1-centos-integration

@dtantsur
Copy link
Member

/approve

@metal3-io-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dtantsur

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@metal3-io-bot metal3-io-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 14, 2022
@elfosardo
Copy link
Member

/lgtm

@metal3-io-bot metal3-io-bot added the lgtm Indicates that a PR is ready to be merged. label Jun 17, 2022
@metal3-io-bot metal3-io-bot merged commit 946dc6d into metal3-io:main Jun 17, 2022
@lentzi90 lentzi90 deleted the lentzi90/no-envsubst-prefix branch June 20, 2022 05:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Clean up ironic deployment manifests
4 participants