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

Add ADR explaining removal of kubebuilder defaults #335

Merged

Conversation

alexander-demicev
Copy link
Contributor

What this PR does / why we need it:

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

@alexander-demicev alexander-demicev added the kind/documentation Improvements or additions to documentation label May 21, 2024
Danil-Grigorev
Danil-Grigorev previously approved these changes May 21, 2024
docs/adr/0002-deprecate-kubebuilder-defaults.md Outdated Show resolved Hide resolved
Signed-off-by: Alexandr Demicev <alexandr.demicev@suse.com>
Copy link
Contributor

@salasberryfin salasberryfin left a comment

Choose a reason for hiding this comment

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

This is just a question and not a blocker for merging the PR. Can this deprecation of default values cause an issue for users that have been using older versions of the provider? Let's say that a user provisions clusters via a GitOps tool (like Fleet) and they are missing some of the fields that no longer have a default value. Now they update CAPRKE2 to the new version that requires to set fields manually. Is it possible that this breaks their downstream cluster/s?

@Danil-Grigorev
Copy link
Contributor

@salasberryfin Defaults are only evaluated on the first apply, then the applied data is stored in etcd, so all existing manifests should already have user defined values for these fields.

@salasberryfin
Copy link
Contributor

Thanks @Danil-Grigorev.

@alexander-demicev alexander-demicev merged commit 3c996d2 into rancher-sandbox:main May 21, 2024
7 checks passed
@alexander-demicev alexander-demicev deleted the v1beta1adr branch May 21, 2024 14:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants