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

fix(flux2): add variable to customise cluster domain (#139) #140

Merged
merged 11 commits into from
Nov 5, 2022

Conversation

dmccaffery
Copy link
Collaborator

  • add new clusterDomain variable which defaults to cluster.local
  • use clusterDomain for source-controller storage-adv-addr argument
  • use clusterDomain for events-addr argument

NOTES:

The source-controller manifest included a hard-coded advertising address with cluster.local. In some environments, the cluster domain is modified, which broke the kustomize controller from being able to resolve the source controller to acquire artifacts.

BREAKING CHANGE:

The eventsaddr variable has been removed as it is no longer necessary. The new clusterDomain variable is now used to create a fully-qualified addresses throughout the deployment manifiests.

Signed-off-by: Deavon M. McCaffery [email protected]

What this PR does / why we need it:

Which issue this PR fixes

(optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged)

Special notes for your reviewer:

Checklist

  • DCO signed
  • Chart Version bumped
  • helm-docs are updated
  • Helm chart is tested
  • Update artifacthub.io/changes in Chart.yaml
  • Run make reviewable

…y#139)

* add new `clusterDomain` variable which defaults to `cluster.local`
* use `clusterDomain` for source-controller `storage-adv-addr` argument
* use `clusterDomain` for `events-addr` argument

NOTES:

The source-controller manifest included a hard-coded advertising
address with `cluster.local`. In some environments, the cluster domain
is modified, which broke the kustomize controller from being able to
resolve the source controller to acquire artifacts.

BREAKING CHANGE:

The `eventsaddr` variable has been removed as it is no longer necessary.
The new `clusterDomain` variable is now used to create a fully-qualified
addresses throughout the deployment manifiests.

Signed-off-by: Deavon M. McCaffery <[email protected]>
@stefanprodan
Copy link
Member

If we are making breaking changes I would suggest correcting the variables names, some of them are not capitalized correctly for example serviceaccount should be serviceAccount, additionalargs should be additionalArgs, watchallnamespaces should be watchAllNamespaces.

@dmccaffery
Copy link
Collaborator Author

I agree. It drives me nuts. 🙃

@dmccaffery dmccaffery enabled auto-merge (rebase) November 5, 2022 15:02
@dmccaffery
Copy link
Collaborator Author

@stefanprodan I fixed the casing of everything that I could find. I've left them each in separate commits to make them easier to review. Per the contributing guide, changes should be squashed before opening a PR, but I figured that would make this much more difficult to review.

Copy link
Member

@stefanprodan stefanprodan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Thanks @dmccaffery 🥇

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.

bug: we should not assume that the cluster domain is cluster.local
2 participants