Skip to content

Commit

Permalink
fix: fix ControlPlane managed resource labels during migration from 1…
Browse files Browse the repository at this point in the history 
….2 and older (#369)

* fix: fix ControlPlane managed resource labels during migration from older operator versions

* chore: derp

* Update controller/controlplane/controller_watch.go

Co-authored-by: Jakub Warczarek <jakub.warczarek@konghq.com>

* chore: fix missing labels var

* Apply suggestions from code review

Co-authored-by: Mattia Lavacca <lavacca.mattia@gmail.com>

* chore: add comments with issue ref

---------

Co-authored-by: Jakub Warczarek <jakub.warczarek@konghq.com>
Co-authored-by: Mattia Lavacca <lavacca.mattia@gmail.com>
  • Loading branch information
@pmalek @programmer04 @mlavacca
3 people authored Jul 10, 2024
1 parent ec25b89 commit 508daaf
Show file tree
Hide file tree
Showing 14 changed files with 595 additions and 85 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,12 @@
- Proper `User-Agent` header is now set on outgoing HTTP requests.
[#387](https://github.com/Kong/gateway-operator/pull/387)

### Fixed

- Fixed `ControlPlane` cluster wide resources not migrating to new ownership labels
(introduced in 1.3.0) when upgrading the operator form 1.2 (or older) to 1.3.0.
[#369](https://github.com/Kong/gateway-operator/pull/369)

## [v1.3.0]

> Release date: 2024-06-24
Expand Down
3 changes: 0 additions & 3 deletions controller/controlplane/controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,6 @@ func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error {
validatinWebhookConfigurationOwnerPredicate.UpdateFunc = func(e event.UpdateEvent) bool {
return r.validatingWebhookConfigurationHasControlPlaneOwner(e.ObjectOld)
}
validatinWebhookConfigurationOwnerPredicate.DeleteFunc = func(e event.DeleteEvent) bool {
return r.validatingWebhookConfigurationHasControlPlaneOwner(e.Object)
}

return ctrl.NewControllerManagedBy(mgr).
// watch ControlPlane objects
Expand Down
174 changes: 154 additions & 20 deletions controller/controlplane/controller_reconciler_utils.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -293,16 +293,44 @@ func (r *Reconciler) ensureClusterRole(
ctx context.Context,
cp *operatorv1beta1.ControlPlane,
) (createdOrUpdated bool, cr *rbacv1.ClusterRole, err error) {
managedByLabelSet := k8sutils.GetManagedByLabelSet(cp)
// NOTE: Code below performs a migration from the old managedBy label to the new one.
// It lists both resources labeled with the old and the new label set and merges them,
// then it reduces the number of resources to 1 and eventually updates the resource.
// After several versions of soak time we can remove handling the legacy label set.
// PR that introduced the new set of labels: https://github.com/Kong/gateway-operator/pull/259
// PR that introduced the migration: https://github.com/Kong/gateway-operator/pull/369
// TODO: https://github.com/Kong/gateway-operator/issues/401.
clusterRolesLegacy, err := k8sutils.ListClusterRoles(
ctx,
r.Client,
client.MatchingLabels(k8sutils.GetLegacyManagedByLabelSet(cp)),
)
if err != nil {
return false, nil, err
}

clusterRoles, err := k8sutils.ListClusterRoles(
ctx,
r.Client,
client.MatchingLabels(managedByLabelSet),
client.MatchingLabels(k8sutils.GetManagedByLabelSet(cp)),
)
if err != nil {
return false, nil, err
}

// NOTE: This is a temporary workaround to handle the migration from the old managedBy label to the new one.
// The reason for this is because those label sets overlap.
for i, cr := range clusterRolesLegacy {
nn := client.ObjectKeyFromObject(&cr)
if lo.ContainsBy(clusterRoles, func(iterator rbacv1.ClusterRole) bool {
return client.ObjectKeyFromObject(&iterator) == nn
}) {
continue
}

clusterRoles = append(clusterRoles, clusterRolesLegacy[i])
}

count := len(clusterRoles)
if count > 1 {
if err := k8sreduce.ReduceClusterRoles(ctx, r.Client, clusterRoles); err != nil {
Expand Down Expand Up @@ -350,17 +378,44 @@ func (r *Reconciler) ensureClusterRoleBinding(
) (createdOrUpdate bool, crb *rbacv1.ClusterRoleBinding, err error) {
logger := log.GetLogger(ctx, "controlplane.ensureClusterRoleBinding", r.DevelopmentMode)

managedByLabelSet := k8sutils.GetManagedByLabelSet(cp)
// NOTE: Code below performs a migration from the old managedBy label to the new one.
// It lists both resources labeled with the old and the new label set and merges them,
// then it reduces the number of resources to 1 and eventually updates the resource.
// After several versions of soak time we can remove handling the legacy label set.
// PR that introduced the new set of labels: https://github.com/Kong/gateway-operator/pull/259
// PR that introduced the migration: https://github.com/Kong/gateway-operator/pull/369
// TODO: https://github.com/Kong/gateway-operator/issues/401.
clusterRoleBindingsLegacy, err := k8sutils.ListClusterRoleBindings(
ctx,
r.Client,
client.MatchingLabels(k8sutils.GetLegacyManagedByLabelSet(cp)),
)
if err != nil {
return false, nil, err
}

clusterRoleBindings, err := k8sutils.ListClusterRoleBindings(
ctx,
r.Client,
client.MatchingLabels(managedByLabelSet),
client.MatchingLabels(k8sutils.GetManagedByLabelSet(cp)),
)
if err != nil {
return false, nil, err
}

// NOTE: This is a temporary workaround to handle the migration from the old managedBy label to the new one.
// The reason for this is because those label sets overlap.
for i, crb := range clusterRoleBindingsLegacy {
nn := client.ObjectKeyFromObject(&crb)
if lo.ContainsBy(clusterRoleBindings, func(iterator rbacv1.ClusterRoleBinding) bool {
return client.ObjectKeyFromObject(&iterator) == nn
}) {
continue
}

clusterRoleBindings = append(clusterRoleBindings, clusterRoleBindingsLegacy[i])
}

count := len(clusterRoleBindings)
if count > 1 {
if err := k8sreduce.ReduceClusterRoleBindings(ctx, r.Client, clusterRoleBindings); err != nil {
Expand Down Expand Up @@ -503,24 +558,39 @@ func (r *Reconciler) ensureOwnedClusterRolesDeleted(
ctx context.Context,
cp *operatorv1beta1.ControlPlane,
) (deletions bool, err error) {
managedByLabelSet := k8sutils.GetManagedByLabelSet(cp)
// TODO: Remove listing with old labels https://github.com/Kong/gateway-operator/issues/401.
clusterRolesLegacy, err := k8sutils.ListClusterRoles(
ctx,
r.Client,
client.MatchingLabels(k8sutils.GetLegacyManagedByLabelSet(cp)),
)
if err != nil {
return false, err
}

clusterRoles, err := k8sutils.ListClusterRoles(
ctx, r.Client,
client.MatchingLabels(managedByLabelSet),
ctx,
r.Client,
client.MatchingLabels(k8sutils.GetManagedByLabelSet(cp)),
)
if err != nil {
return false, err
}

clusterRoles = append(clusterRoles, clusterRolesLegacy...)

var (
deleted bool
errs []error
)
for i := range clusterRoles {
err = r.Client.Delete(ctx, &clusterRoles[i])
if err != nil && !k8serrors.IsNotFound(err) {
if err != nil {
if k8serrors.IsNotFound(err) {
continue
}
errs = append(errs, err)
continue
}
deleted = true
}
Expand All @@ -535,24 +605,39 @@ func (r *Reconciler) ensureOwnedClusterRoleBindingsDeleted(
ctx context.Context,
cp *operatorv1beta1.ControlPlane,
) (deletions bool, err error) {
managedByLabelSet := k8sutils.GetManagedByLabelSet(cp)
// TODO: Remove listing with old labels https://github.com/Kong/gateway-operator/issues/401.
clusterRoleBindingsLegacy, err := k8sutils.ListClusterRoleBindings(
ctx,
r.Client,
client.MatchingLabels(k8sutils.GetLegacyManagedByLabelSet(cp)),
)
if err != nil {
return false, err
}

clusterRoleBindings, err := k8sutils.ListClusterRoleBindings(
ctx, r.Client,
client.MatchingLabels(managedByLabelSet),
ctx,
r.Client,
client.MatchingLabels(k8sutils.GetManagedByLabelSet(cp)),
)
if err != nil {
return false, err
}

clusterRoleBindings = append(clusterRoleBindings, clusterRoleBindingsLegacy...)

var (
deleted bool
errs []error
)
for i := range clusterRoleBindings {
err = r.Client.Delete(ctx, &clusterRoleBindings[i])
if err != nil && !k8serrors.IsNotFound(err) {
if err != nil {
if k8serrors.IsNotFound(err) {
continue
}
errs = append(errs, err)
continue
}
deleted = true
}
Expand All @@ -561,26 +646,44 @@ func (r *Reconciler) ensureOwnedClusterRoleBindingsDeleted(
}

func (r *Reconciler) ensureOwnedValidatingWebhookConfigurationDeleted(ctx context.Context,
cp *operatorv1beta1.ControlPlane) (deletions bool, err error) {
managedByLabelSet := k8sutils.GetManagedByLabelSet(cp)
cp *operatorv1beta1.ControlPlane,
) (deletions bool, err error) {
// TODO: Remove listing with old labels and owner ref https://github.com/Kong/gateway-operator/issues/401.
validatingWebhookConfigurationsLegacy, err := k8sutils.ListValidatingWebhookConfigurationsForOwner(
ctx,
r.Client,
cp.GetUID(),
// NOTE: this uses only the 1 label to find the legacy webhook configurations not the label set
// because app:<name> is not set on ValidatingWebhookConfiguration.
client.MatchingLabels(k8sutils.GetLegacyManagedByLabel(cp)),
)
if err != nil {
return false, fmt.Errorf("failed listing webhook configurations for owner: %w", err)
}

validatingWebhookConfigurations, err := k8sutils.ListValidatingWebhookConfigurations(
ctx,
r.Client,
client.MatchingLabels(managedByLabelSet),
client.MatchingLabels(k8sutils.GetManagedByLabelSet(cp)),
)
if err != nil {
return false, fmt.Errorf("failed listing webhook configurations for owner: %w", err)
}

validatingWebhookConfigurations = append(validatingWebhookConfigurations, validatingWebhookConfigurationsLegacy...)

var (
deleted bool
errs []error
)
for i := range validatingWebhookConfigurations {
err = r.Client.Delete(ctx, &validatingWebhookConfigurations[i])
if err != nil && !k8serrors.IsNotFound(err) {
if err != nil {
if k8serrors.IsNotFound(err) {
continue
}
errs = append(errs, err)
continue
}
deleted = true
}
Expand Down Expand Up @@ -671,15 +774,45 @@ func (r *Reconciler) ensureValidatingWebhookConfiguration(
) (op.Result, error) {
logger := log.GetLogger(ctx, "controlplane.ensureValidatingWebhookConfiguration", r.DevelopmentMode)

managedByLabelSet := k8sutils.GetManagedByLabelSet(cp)
// NOTE: Code below performs a migration from the old managedBy label to the new one.
// It lists both resources labeled with the old and the new label set and merges them,
// then it reduces the number of resources to 1 and eventually updates the resource.
// After several versions of soak time we can remove handling the legacy label set.
// PR that introduced the new set of labels: https://github.com/Kong/gateway-operator/pull/259
// PR that introduced the migration: https://github.com/Kong/gateway-operator/pull/369
// TODO: https://github.com/Kong/gateway-operator/issues/401.
validatingWebhookConfigurationsLegacy, err := k8sutils.ListValidatingWebhookConfigurationsForOwner(
ctx,
r.Client,
cp.GetUID(),
// NOTE: this uses only the 1 label to find the legacy webhook configurations not the label set
// because app:<name> is not set on ValidatingWebhookConfiguration.
client.MatchingLabels(k8sutils.GetLegacyManagedByLabel(cp)),
)
if err != nil {
return op.Noop, fmt.Errorf("failed listing webhook configurations for owner: %w", err)
}

validatingWebhookConfigurations, err := k8sutils.ListValidatingWebhookConfigurations(
ctx,
r.Client,
client.MatchingLabels(managedByLabelSet),
client.MatchingLabels(k8sutils.GetManagedByLabelSet(cp)),
)
if err != nil {
return op.Noop, err
return op.Noop, fmt.Errorf("failed listing webhook configurations for owner: %w", err)
}

// NOTE: This is a temporary workaround to handle the migration from the old managedBy label to the new one.
// The reason for this is because those label sets overlap.
for i, vwc := range validatingWebhookConfigurationsLegacy {
nn := client.ObjectKeyFromObject(&vwc)
if lo.ContainsBy(validatingWebhookConfigurations, func(iterator admregv1.ValidatingWebhookConfiguration) bool {
return client.ObjectKeyFromObject(&iterator) == nn
}) {
continue
}

validatingWebhookConfigurations = append(validatingWebhookConfigurations, validatingWebhookConfigurationsLegacy[i])
}

count := len(validatingWebhookConfigurations)
Expand Down Expand Up @@ -736,7 +869,8 @@ func (r *Reconciler) ensureValidatingWebhookConfiguration(

updated, webhookConfiguration.ObjectMeta = k8sutils.EnsureObjectMetaIsUpdated(webhookConfiguration.ObjectMeta, generatedWebhookConfiguration.ObjectMeta)

if !cmp.Equal(webhookConfiguration.Webhooks, generatedWebhookConfiguration.Webhooks) {
if !cmp.Equal(webhookConfiguration.Webhooks, generatedWebhookConfiguration.Webhooks) ||
!cmp.Equal(webhookConfiguration.Labels, generatedWebhookConfiguration.Labels) {
webhookConfiguration.Webhooks = generatedWebhookConfiguration.Webhooks
updated = true
}
Expand Down
Loading

0 comments on commit 508daaf

Please sign in to comment.