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.

Sign up for GitHub

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

Jump to bottom

Add ManagedByLabel to user ClusterBackup resources #13396

Merged
merged 2 commits into from
May 21, 2024
Merged

Add ManagedByLabel to user ClusterBackup resources #13396

merged 2 commits into from
May 21, 2024

Conversation

moelsayed
Copy link
Contributor

What this PR does / why we need it:
This PR fixes a bug where Velero CRDs are removed from user clusters if it's deployed externally and Cluster Backup is disabled.
Which issue(s) this PR fixes:

Fixes #13393

What type of PR is this?

/kind bug
Special notes for your reviewer:

Does this PR introduce a user-facing change? Then add your Release Note here:

Fixes #13393 where externally deployed Velero CRDs are removed automatically from user user cluster.

Documentation:

NONE

@kubermatic-bot kubermatic-bot added docs/none Denotes a PR that doesn't need documentation (changes). release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/bug Categorizes issue or PR as related to a bug. dco-signoff: yes Denotes that all commits in the pull request have the valid DCO signoff message. sig/cluster-management Denotes a PR or issue as being assigned to SIG Cluster Management. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels May 17, 2024
embik
embik requested changes May 20, 2024
View reviewed changes
pkg/resources/resources.go Outdated
@@ -1071,6 +1072,7 @@ const (
ClusterBackupUsername = "velero"
ClusterBackupServiceAccountName = "velero"
ClusterBackupNamespaceName = "velero"
ClusterBakcupControllerName = "cluster-backup-controller"
Copy link
Member

Choose a reason for hiding this comment

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

Typo.

Suggested change
ClusterBakcupControllerName = "cluster-backup-controller"
ClusterBackupControllerName = "cluster-backup-controller"

embik
embik reviewed May 20, 2024
View reviewed changes
pkg/ee/cluster-backup/controller.go Outdated
@@ -323,6 +324,7 @@ func (r *reconciler) undeployClusterBackupUserClusterCRDs(ctx context.Context, u
LabelSelector: labels.SelectorFromSet(
map[string]string{
"component": "velero",
appskubermaticv1.ApplicationManagedByLabel: resources.ClusterBakcupControllerName,
Copy link
Member

Choose a reason for hiding this comment

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

This should apply to the other resources as well, else we'll still delete Deployments and DaemonSets we never managed.

@moelsayed
Copy link
Contributor Author

/test pre-kubermatic-etcd-launcher-e2e

@xrstf
Copy link
Contributor

xrstf commented May 21, 2024

/approve
/lgtrm

Personally I would not have put a controller name constant in the resources package, cause that's not where it belongs, and would have used an objectModifier to apply the labels instead of editing every single reconciler function to do it manually (and therereby getting rid of the new "need" to have the controller name in a completely foreign, remote, far away package), but nwe're in a hurry, so I guess... let's merge it.

@kubermatic-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: xrstf

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@kubermatic-bot kubermatic-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 21, 2024
@xrstf
Copy link
Contributor

xrstf commented May 21, 2024

/lgtm
/cherrypick release/v2.25

@kubermatic-bot
Copy link
Contributor

@xrstf: once the present PR merges, I will cherry-pick it on top of release/v2.25 in a new PR and assign it to you.

In response to this:

/lgtm
/cherrypick release/v2.25

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@kubermatic-bot kubermatic-bot added the lgtm Indicates that a PR is ready to be merged. label May 21, 2024
@kubermatic-bot
Copy link
Contributor

LGTM label has been added.

Git tree hash: 9b62bc9b5e1d7f0d5e0acee50696bb27b079f4d4

@kubermatic-bot kubermatic-bot merged commit d668558 into kubermatic:main May 21, 2024
18 checks passed
@kubermatic-bot kubermatic-bot added this to the KKP 2.26 milestone May 21, 2024
@kubermatic-bot kubermatic-bot mentioned this pull request May 21, 2024
Merged
@kubermatic-bot
Copy link
Contributor

@xrstf: new pull request created: #13397

In response to this:

/lgtm
/cherrypick release/v2.25

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@xrstf xrstf mentioned this pull request May 22, 2024
Merged
@embik embik added the backport-complete Denotes a PR or issue which has been fully backported to all required release branches. label Jun 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@embik embik embik requested changes

Assignees

@xrstf xrstf

@embik embik

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. backport-complete Denotes a PR or issue which has been fully backported to all required release branches. dco-signoff: yes Denotes that all commits in the pull request have the valid DCO signoff message. docs/none Denotes a PR that doesn't need documentation (changes). kind/bug Categorizes issue or PR as related to a bug. lgtm Indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/cluster-management Denotes a PR or issue as being assigned to SIG Cluster Management. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
KKP 2.26
Development

Successfully merging this pull request may close these issues.

Velero CRDs are auto-removed
4 participants
@moelsayed @xrstf @kubermatic-bot @embik