Skip to content

Commit

Permalink
Remove the migration step to add labels to creds secret for (#400)
Browse files Browse the repository at this point in the history
ToolchainClusters.
  • Loading branch information
metlos authored Jun 10, 2024
1 parent 6a80f65 commit b4e25fa
Show file tree
Hide file tree
Showing 2 changed files with 0 additions and 89 deletions.
41 changes: 0 additions & 41 deletions controllers/toolchaincluster/toolchaincluster_controller.go
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
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

0 comments on commit b4e25fa

Please sign in to comment.