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

Remove the migration step to add labels to creds secret for ToolchainClusters. #400

Merged
merged 2 commits into from
Jun 10, 2024
Merged

Remove the migration step to add labels to creds secret for ToolchainClusters. #400

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7b4f6f8
Remove the migration step to add labels to creds secret for
metlos Jun 5, 2024
4629d93
Merge branch 'master' into remove-migration-to-add-secret-labels
metlos Jun 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 0 additions & 41 deletions controllers/toolchaincluster/toolchaincluster_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (

toolchainv1alpha1 "github.com/codeready-toolchain/api/api/v1alpha1"
"github.com/codeready-toolchain/toolchain-common/pkg/cluster"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/runtime"
kubeclientset "k8s.io/client-go/kubernetes"
Expand Down Expand Up @@ -52,14 +51,6 @@ func (r *Reconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl.
return reconcile.Result{}, err
}

// this is a migration step to make sure that we are forwards-compatible with
// the secrets labeled by the toolchainCluster name, which are going to be the basis
// for *creating* toolchain clusters in the future.
if err := r.labelTokenSecret(ctx, toolchainCluster); err != nil {
reqLogger.Error(err, "unable to check the labels in the associated secret")
return reconcile.Result{}, err
}

cachedCluster, ok := cluster.GetCachedToolchainCluster(toolchainCluster.Name)
if !ok {
err := fmt.Errorf("cluster %s not found in cache", toolchainCluster.Name)
Expand Down Expand Up @@ -89,35 +80,3 @@ func (r *Reconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl.

return reconcile.Result{RequeueAfter: r.RequeAfter}, nil
}

func (r *Reconciler) labelTokenSecret(ctx context.Context, toolchainCluster *toolchainv1alpha1.ToolchainCluster) error {
if toolchainCluster.Spec.SecretRef.Name == "" {
return nil
}

secret := &corev1.Secret{}
if err := r.Client.Get(ctx, client.ObjectKey{Name: toolchainCluster.Spec.SecretRef.Name, Namespace: toolchainCluster.Namespace}, secret); err != nil {
if errors.IsNotFound(err) {
// The referenced secret does not exist yet, so we can't really label it.
// Because the reconciler runs periodically (not just on ToolchainCluster change), we will
// recover from this condition once the secret appears in the cluster.
log.FromContext(ctx).Info("failed to find the referenced secret. Cluster cache might be broken until it is created.", "expectedSecretName", toolchainCluster.Spec.SecretRef.Name)
return nil
}
return err
}

if secret.Labels[toolchainv1alpha1.ToolchainClusterLabel] != toolchainCluster.Name {
if secret.Labels == nil {
secret.Labels = map[string]string{}
}

secret.Labels[toolchainv1alpha1.ToolchainClusterLabel] = toolchainCluster.Name

if err := r.Client.Update(ctx, secret); err != nil {
return err
}
}

return nil
}
48 changes: 0 additions & 48 deletions controllers/toolchaincluster/toolchaincluster_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
toolchainv1alpha1 "github.com/codeready-toolchain/api/api/v1alpha1"
"github.com/codeready-toolchain/toolchain-common/pkg/cluster"
"github.com/codeready-toolchain/toolchain-common/pkg/test"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"gopkg.in/h2non/gock.v1"
corev1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -120,53 +119,6 @@ func TestClusterControllerChecks(t *testing.T) {
require.NoError(t, err)
assertClusterStatus(t, cl, "stable", offline())
})

t.Run("pre-existing secret is updated with the label linking it to the toolchaincluster resource", func(t *testing.T) {
// given
tc, secret := newToolchainCluster("tc", tcNs, "http://cluster.com", toolchainv1alpha1.ToolchainClusterStatus{})

cl := test.NewFakeClient(t, tc, secret)
reset := setupCachedClusters(t, cl, tc)
defer reset()
controller, req := prepareReconcile(tc, cl, requeAfter)

// just make sure that there is label on the secret yet...
require.Empty(t, secret.Labels[toolchainv1alpha1.ToolchainClusterLabel])

// when
_, err := controller.Reconcile(context.TODO(), req)

// then
require.NoError(t, err)
linkedSecret := &corev1.Secret{}
err = cl.Client.Get(context.TODO(), types.NamespacedName{Name: tc.Spec.SecretRef.Name, Namespace: tcNs}, linkedSecret)
require.NoError(t, err)
assert.Equal(t, "tc", linkedSecret.Labels[toolchainv1alpha1.ToolchainClusterLabel])
})

t.Run("secret labeling does not break on missing secret even though the missing secret breaks the tc cache", func(t *testing.T) {
// given
stable, secret := newToolchainCluster("stable", tcNs, "http://cluster.com", toolchainv1alpha1.ToolchainClusterStatus{})

// we need the secret to be able to initialize the cluster cache
cl := test.NewFakeClient(t, stable, secret)

controller, req := prepareReconcile(stable, cl, requeAfter)
// initialize the cluster cache at the point in time we still have the secret
reset := setupCachedClusters(t, cl, stable)
defer reset()

// now enter the invalid state - delete the secret before the actual reconcile and check that we don't get an error.
// we don't care here that the cluster is essentially in an invalid state because all we test here is that the labeling
// doesn't introduce a new failure mode.
require.NoError(t, cl.Delete(context.TODO(), secret))

// when
_, err := controller.Reconcile(context.TODO(), req)

// then
require.NoError(t, err)
})
}

func setupCachedClusters(t *testing.T, cl *test.FakeClient, clusters ...*toolchainv1alpha1.ToolchainCluster) func() {
Expand Down
Loading