-
Notifications
You must be signed in to change notification settings - Fork 9
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
fix: Namespaced deployment overlay generation #301
Conversation
* Remove dnshealthcheckprobes CRD for namespaced deployments * Add crd to cluster kustomization * Improve dns provider generation, avoid calling a base that requires a local file. Signed-off-by: Michael Nairn <[email protected]>
Signed-off-by: Michael Nairn <[email protected]>
Test was incorrectly checking for "GEO-" in the aws provider specific endpoint values. Signed-off-by: Michael Nairn <[email protected]>
a0396b0
to
5b3a166
Compare
github action sounds good |
@@ -8,3 +8,9 @@ patches: | |||
kind: CustomResourceDefinition | |||
metadata: | |||
name: dnsrecords.kuadrant.io | |||
- patch: |- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not getting why this is needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ../../deploy/local
kustomization which this is modifying adds the crds which prevents you from using it multiple times in an overlay. When we install multiple instances of the dns operator we can use this kustomization which removes the CRDs multiple times for each namespace we want to install into, and install the crds separately once.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm just not sure why we have to remove the probe crd?
fix: Namespaced deployment overlay generation
bump kustomize version v5.5.0
fix: Multi record test
Test was incorrectly checking for "GEO-" in the aws provider specific endpoint values.
Note: This test is executed on the e2e but only with 1 cluster/deployment so it never uses the second geo, you can re-create the issue by running:
We should probably consider adding a GitHub action that executes it this way.