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

feat(domain): add option to set controller domain #52

Merged
merged 1 commit into from
Nov 15, 2024

Conversation

akosveres
Copy link
Collaborator

  • Add new command line flag for a controller domain
  • Update docs

flag.StringVar(&metricsAddr, "metrics-bind-address", ":8080", "The address the metric endpoint binds to.")
flag.StringVar(&probeAddr, "health-probe-bind-address", ":8081", "The address the probe endpoint binds to.")
flag.BoolVar(&enableLeaderElection, "leader-elect", false,
"Enable leader election for controller manager. "+
"Enabling this will ensure there is only one active controller manager.")
flag.StringVar(&controllerDomain, "controller-domain", "k8s.checklyhq.com", "Domain to use for annotations and finalizers.")
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This may not be the best naming of this ... but it's here, so please, suggest better ones and I'll update the PR.

Copy link

Choose a reason for hiding this comment

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

I've asked our k8s users for their opinion, but to me (as a lightweight user) it seems alright.

Copy link

Choose a reason for hiding this comment

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

They are saying it's good 👍

Choose a reason for hiding this comment

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Choose a reason for hiding this comment

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

I mean if the controller-domain is the same concept as the prefix mentioned here

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, @pelzerim, correct, that's exactly what this is supposed to be, I just never seen this doc :D

config/manager/manager.yaml Outdated Show resolved Hide resolved
docs/README.md Show resolved Hide resolved
* Add new command line flag for a controller domain
* Update docs
Copy link

@sorccu sorccu left a comment

Choose a reason for hiding this comment

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

Changes look good and straightforward to me.

@akosveres akosveres merged commit b308dea into main Nov 15, 2024
2 checks passed
@akosveres akosveres deleted the controller_domain branch November 15, 2024 09:37
Copy link

🎉 This PR is included in version 1.7.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@alen-z
Copy link

alen-z commented Nov 15, 2024

This could be used to work with different operator deployment, which is great. Alternative is to use version value on whichever domain. Here I see option to use v1.checklyhq.com and v2.checklyhq.com which could do the trick. Each operator would look at its specified version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants