Skip to content

Commit c76780d

Browse files
thardeckthardeck
committed
Remove deprecated created-by label (#4225)
It was added by Rancher UI but was redundant because of the additional label `created-by-user-id`.
1 parent 9dec55d commit c76780d

File tree

5 files changed

+229
-2
lines changed

5 files changed

+229
-2
lines changed

‎integrationtests/controller/bundle/label_migration_test.go‎

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
package bundle
2+
3+
import (
4+
. "github.com/onsi/ginkgo/v2"
5+
. "github.com/onsi/gomega"
6+
7+
"github.com/rancher/fleet/integrationtests/utils"
8+
"github.com/rancher/fleet/pkg/apis/fleet.cattle.io/v1alpha1"
9+
10+
corev1 "k8s.io/api/core/v1"
11+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
12+
"k8s.io/apimachinery/pkg/types"
13+
)
14+
15+
var _ = Describe("Bundle label migration", func() {
16+
BeforeEach(func() {
17+
var err error
18+
namespace, err = utils.NewNamespaceName()
19+
Expect(err).ToNot(HaveOccurred())
20+
Expect(k8sClient.Create(ctx, &corev1.Namespace{
21+
ObjectMeta: metav1.ObjectMeta{Name: namespace},
22+
})).ToNot(HaveOccurred())
23+
24+
_, err = utils.CreateCluster(ctx, k8sClient, "cluster", namespace, nil, namespace)
25+
Expect(err).NotTo(HaveOccurred())
26+
27+
DeferCleanup(func() {
28+
Expect(k8sClient.Delete(ctx, &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: namespace}})).ToNot(HaveOccurred())
29+
})
30+
})
31+
32+
createBundle := func(name string, labels map[string]string) {
33+
bundle := &v1alpha1.Bundle{
34+
ObjectMeta: metav1.ObjectMeta{
35+
Name: name,
36+
Namespace: namespace,
37+
Labels: labels,
38+
},
39+
Spec: v1alpha1.BundleSpec{
40+
BundleDeploymentOptions: v1alpha1.BundleDeploymentOptions{
41+
DefaultNamespace: "default",
42+
},
43+
Targets: []v1alpha1.BundleTarget{
44+
{
45+
BundleDeploymentOptions: v1alpha1.BundleDeploymentOptions{
46+
TargetNamespace: "targetNs",
47+
},
48+
Name: "cluster",
49+
ClusterName: "cluster",
50+
},
51+
},
52+
},
53+
}
54+
Expect(k8sClient.Create(ctx, bundle)).ToNot(HaveOccurred())
55+
}
56+
57+
DescribeTable("should remove deprecated label after migration",
58+
func(bundleName string, initialLabels map[string]string) {
59+
const deprecatedLabel = "fleet.cattle.io/created-by-display-name"
60+
61+
createBundle(bundleName, initialLabels)
62+
DeferCleanup(func() {
63+
Expect(k8sClient.Delete(ctx, &v1alpha1.Bundle{
64+
ObjectMeta: metav1.ObjectMeta{Name: bundleName, Namespace: namespace},
65+
})).ToNot(HaveOccurred())
66+
})
67+
68+
Eventually(func(g Gomega) {
69+
bundle := &v1alpha1.Bundle{}
70+
g.Expect(k8sClient.Get(ctx, types.NamespacedName{Namespace: namespace, Name: bundleName}, bundle)).To(Succeed())
71+
g.Expect(bundle.Status.ObservedGeneration).To(BeNumerically(">", 0))
72+
}).Should(Succeed())
73+
74+
bundle := &v1alpha1.Bundle{}
75+
Expect(k8sClient.Get(ctx, types.NamespacedName{Namespace: namespace, Name: bundleName}, bundle)).To(Succeed())
76+
77+
Expect(bundle.Labels).ToNot(HaveKey(deprecatedLabel))
78+
Expect(bundle.Labels).To(HaveKey(v1alpha1.CreatedByUserIDLabel))
79+
},
80+
Entry("with label present initially",
81+
"bundle-with-label",
82+
map[string]string{
83+
"fleet.cattle.io/created-by-display-name": "admin",
84+
v1alpha1.CreatedByUserIDLabel: "user-12345",
85+
},
86+
),
87+
Entry("without label present initially",
88+
"bundle-without-label",
89+
map[string]string{
90+
v1alpha1.CreatedByUserIDLabel: "user-12345",
91+
},
92+
),
93+
)
94+
})

‎integrationtests/gitjob/controller/label_migration_test.go‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
package controller
2+
3+
import (
4+
. "github.com/onsi/ginkgo/v2"
5+
. "github.com/onsi/gomega"
6+
7+
"github.com/rancher/fleet/integrationtests/utils"
8+
"github.com/rancher/fleet/pkg/apis/fleet.cattle.io/v1alpha1"
9+
10+
corev1 "k8s.io/api/core/v1"
11+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
12+
"k8s.io/apimachinery/pkg/types"
13+
)
14+
15+
var _ = Describe("GitRepo label migration", func() {
16+
var namespace string
17+
18+
BeforeEach(func() {
19+
var err error
20+
namespace, err = utils.NewNamespaceName()
21+
Expect(err).ToNot(HaveOccurred())
22+
Expect(k8sClient.Create(ctx, &corev1.Namespace{
23+
ObjectMeta: metav1.ObjectMeta{Name: namespace},
24+
})).ToNot(HaveOccurred())
25+
26+
DeferCleanup(func() {
27+
Expect(k8sClient.Delete(ctx, &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: namespace}})).ToNot(HaveOccurred())
28+
})
29+
})
30+
31+
createGitRepo := func(name string, labels map[string]string) {
32+
gitrepo := &v1alpha1.GitRepo{
33+
ObjectMeta: metav1.ObjectMeta{
34+
Name: name,
35+
Namespace: namespace,
36+
Labels: labels,
37+
},
38+
Spec: v1alpha1.GitRepoSpec{
39+
Repo: "https://github.com/rancher/fleet-test-data/not-found",
40+
},
41+
}
42+
Expect(k8sClient.Create(ctx, gitrepo)).ToNot(HaveOccurred())
43+
}
44+
45+
DescribeTable("should remove deprecated label after migration",
46+
func(gitRepoName string, initialLabels map[string]string) {
47+
const deprecatedLabel = "fleet.cattle.io/created-by-display-name"
48+
49+
createGitRepo(gitRepoName, initialLabels)
50+
DeferCleanup(func() {
51+
Expect(k8sClient.Delete(ctx, &v1alpha1.GitRepo{
52+
ObjectMeta: metav1.ObjectMeta{Name: gitRepoName, Namespace: namespace},
53+
})).ToNot(HaveOccurred())
54+
})
55+
56+
Eventually(func(g Gomega) {
57+
gitrepo := &v1alpha1.GitRepo{}
58+
g.Expect(k8sClient.Get(ctx, types.NamespacedName{Namespace: namespace, Name: gitRepoName}, gitrepo)).To(Succeed())
59+
g.Expect(gitrepo.Status.ObservedGeneration).To(BeNumerically(">", 0))
60+
}).Should(Succeed())
61+
62+
gitrepo := &v1alpha1.GitRepo{}
63+
Expect(k8sClient.Get(ctx, types.NamespacedName{Namespace: namespace, Name: gitRepoName}, gitrepo)).To(Succeed())
64+
65+
Expect(gitrepo.Labels).ToNot(HaveKey(deprecatedLabel))
66+
Expect(gitrepo.Labels).To(HaveKey(v1alpha1.CreatedByUserIDLabel))
67+
},
68+
Entry("with label present initially",
69+
"gitrepo-with-label",
70+
map[string]string{
71+
"fleet.cattle.io/created-by-display-name": "admin",
72+
v1alpha1.CreatedByUserIDLabel: "user-12345",
73+
},
74+
),
75+
Entry("without label present initially",
76+
"gitrepo-without-label",
77+
map[string]string{
78+
v1alpha1.CreatedByUserIDLabel: "user-12345",
79+
},
80+
),
81+
)
82+
})

‎internal/cmd/controller/gitops/reconciler/gitjob_controller.go‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,12 @@ func (r *GitJobReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr
199199
return ctrl.Result{RequeueAfter: durations.DefaultRequeueAfter}, nil
200200
}
201201

202+
// Migration: Remove the obsolete created-by-display-name label if it exists
203+
if err := r.removeDisplayNameLabel(ctx, req.NamespacedName); err != nil {
204+
logger.V(1).Error(err, "Failed to remove display name label")
205+
return ctrl.Result{}, err
206+
}
207+
202208
logger = logger.WithValues("generation", gitrepo.Generation, "commit", gitrepo.Status.Commit).WithValues("conditions", gitrepo.Status.Conditions)
203209

204210
if userID := gitrepo.Labels[v1alpha1.CreatedByUserIDLabel]; userID != "" {
@@ -409,6 +415,28 @@ func (r *GitJobReconciler) addGitRepoFinalizer(ctx context.Context, nsName types
409415
return nil
410416
}
411417

418+
// removeDisplayNameLabel removes the obsolete created-by-display-name label from the gitrepo if it exists.
419+
func (r *GitJobReconciler) removeDisplayNameLabel(ctx context.Context, nsName types.NamespacedName) error {
420+
return retry.RetryOnConflict(retry.DefaultRetry, func() error {
421+
gitrepo := &v1alpha1.GitRepo{}
422+
if err := r.Get(ctx, nsName, gitrepo); err != nil {
423+
return err
424+
}
425+
426+
if gitrepo.Labels == nil {
427+
return nil
428+
}
429+
430+
const deprecatedLabel = "fleet.cattle.io/created-by-display-name"
431+
if _, exists := gitrepo.Labels[deprecatedLabel]; !exists {
432+
return nil
433+
}
434+
435+
delete(gitrepo.Labels, deprecatedLabel)
436+
return r.Update(ctx, gitrepo)
437+
})
438+
}
439+
412440
func (r *GitJobReconciler) validateExternalSecretExist(ctx context.Context, gitrepo *v1alpha1.GitRepo) error {
413441
if gitrepo.Spec.HelmSecretNameForPaths != "" {
414442
if err := r.Get(ctx, types.NamespacedName{Namespace: gitrepo.Namespace, Name: gitrepo.Spec.HelmSecretNameForPaths}, &corev1.Secret{}); err != nil {

‎internal/cmd/controller/gitops/reconciler/gitjob_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -522,7 +522,7 @@ func TestReconcile_Error_WhenGetGitJobErrors(t *testing.T) {
522522
mockClient := mocks.NewMockClient(mockCtrl)
523523
mockClient.EXPECT().List(gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes().Return(nil)
524524

525-
mockClient.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Times(1).DoAndReturn(
525+
mockClient.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Times(2).DoAndReturn(
526526
func(ctx context.Context, req types.NamespacedName, gitrepo *fleetv1.GitRepo, opts ...interface{}) error {
527527
gitrepo.Name = gitRepo.Name
528528
gitrepo.Namespace = gitRepo.Namespace
@@ -590,7 +590,7 @@ func TestReconcile_Error_WhenSecretDoesNotExist(t *testing.T) {
590590
mockClient := mocks.NewMockClient(mockCtrl)
591591
mockClient.EXPECT().List(gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes().Return(nil)
592592

593-
mockClient.EXPECT().Get(gomock.Any(), gomock.Any(), &gitRepoPointerMatcher{}, gomock.Any()).Times(2).DoAndReturn(
593+
mockClient.EXPECT().Get(gomock.Any(), gomock.Any(), &gitRepoPointerMatcher{}, gomock.Any()).Times(3).DoAndReturn(
594594
func(ctx context.Context, req types.NamespacedName, gitrepo *fleetv1.GitRepo, opts ...interface{}) error {
595595
gitrepo.Name = gitRepo.Name
596596
gitrepo.Namespace = gitRepo.Namespace

‎internal/cmd/controller/reconciler/bundle_controller.go‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,11 @@ func (r *BundleReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr
214214

215215
bundleOrig := bundle.DeepCopy()
216216

217+
// Migration: Remove the obsolete created-by-display-name label if it exists
218+
if err := r.removeDisplayNameLabel(ctx, bundle); err != nil {
219+
return ctrl.Result{}, err
220+
}
221+
217222
logger.V(1).Info(
218223
"Reconciling bundle, checking targets, calculating changes, building objects",
219224
"generation",
@@ -464,6 +469,24 @@ func (r *BundleReconciler) ensureFinalizer(ctx context.Context, bundle *fleet.Bu
464469
return r.Update(ctx, bundle)
465470
}
466471

472+
// removeDisplayNameLabel removes the obsolete created-by-display-name label from the bundle if it exists.
473+
func (r *BundleReconciler) removeDisplayNameLabel(ctx context.Context, bundle *fleet.Bundle) error {
474+
if bundle.Labels == nil {
475+
return nil
476+
}
477+
478+
const deprecatedLabel = "fleet.cattle.io/created-by-display-name"
479+
if _, exists := bundle.Labels[deprecatedLabel]; !exists {
480+
return nil
481+
}
482+
483+
delete(bundle.Labels, deprecatedLabel)
484+
if err := r.Update(ctx, bundle); err != nil {
485+
return err
486+
}
487+
return nil
488+
}
489+
467490
func (r *BundleReconciler) createBundleDeployment(
468491
ctx context.Context,
469492
logger logr.Logger,

0 commit comments

Comments
 (0)