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

ADR: Clusterctl Config resoruce #713

Merged

Conversation

Danil-Grigorev
Copy link
Contributor

@Danil-Grigorev Danil-Grigorev commented Sep 3, 2024

What this PR does / why we need it:

This change will add a configurability layer on top of the default state of the clusterctl.yaml ConfigMap, exposed to users as a public API.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #

Special notes for your reviewer:

Checklist:

  • squashed commits into logical changes
  • includes documentation
  • adds unit tests
  • adds or updates e2e tests

Copy link
Member

@alexander-demicev alexander-demicev left a comment

Choose a reason for hiding this comment

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

looks good to me, can you also add a couple of sentences to explain that this will be a singleton resource per namespace?

docs/adr/0012-clusterctl-provider.md Outdated Show resolved Hide resolved
@Danil-Grigorev Danil-Grigorev force-pushed the adr-clusterctl-provider branch 2 times, most recently from 98347fa to c12abc6 Compare September 3, 2024 13:44
@Danil-Grigorev Danil-Grigorev changed the title ADR: Clusterctl Provider resoruce ADR: Clusterctl Config resoruce Sep 3, 2024
@Danil-Grigorev Danil-Grigorev force-pushed the adr-clusterctl-provider branch from c12abc6 to 5e39433 Compare September 4, 2024 08:20
@Danil-Grigorev Danil-Grigorev marked this pull request as ready for review September 4, 2024 08:22
@Danil-Grigorev Danil-Grigorev requested a review from a team as a code owner September 4, 2024 08:22
furkatgofurov7
furkatgofurov7 previously approved these changes Sep 4, 2024
Copy link
Contributor

@furkatgofurov7 furkatgofurov7 left a comment

Choose a reason for hiding this comment

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

Small nits, overall LGTM thanks!

docs/adr/0012-clusterctl-provider.md Outdated Show resolved Hide resolved
docs/adr/0012-clusterctl-provider.md Outdated Show resolved Hide resolved
Signed-off-by: Danil-Grigorev <danil.grigorev@suse.com>
@Danil-Grigorev Danil-Grigorev force-pushed the adr-clusterctl-provider branch from e93c079 to ad90ff1 Compare September 4, 2024 09:56
@alexander-demicev alexander-demicev merged commit fc559bb into rancher:main Sep 6, 2024
9 checks passed
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.

3 participants