Skip to content

Commit

Permalink
Don't wait for ControlPlane readiness for secrets.
Browse files Browse the repository at this point in the history
Remove the check for the IsControlPlaneReady in the secret syncing.

This isn't needed and the secret might be valuable for downloading
things like a CNI Helm Chart.
  • Loading branch information
bigkevmcd committed Apr 24, 2023
1 parent 93d3801 commit 26d417e
Show file tree
Hide file tree
Showing 4 changed files with 71 additions and 20 deletions.
3 changes: 3 additions & 0 deletions controllers/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ const (

// IsControlPlaneReady takes a client connected to a cluster and reports whether or
// not the control-plane for the cluster is "ready".
//
// WARNING: This does not work for "managed clusters" where the control-plane is not available.
func IsControlPlaneReady(ctx context.Context, cl client.Client) (bool, error) {
logger := log.FromContext(ctx)
readiness := []bool{}
Expand All @@ -48,6 +50,7 @@ func IsControlPlaneReady(ctx context.Context, cl client.Client) (bool, error) {
}
return true
}

logger.Info("readiness", "len", len(readiness), "is-ready", isReady(readiness))

// If we have no statuses, then we really don't know if we're ready or not.
Expand Down
1 change: 1 addition & 0 deletions controllers/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,5 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client"
)

// ConfigParser functions parse a byte slice and return a Kubernetes client.
type ConfigParser func(b []byte) (client.Client, error)
12 changes: 0 additions & 12 deletions controllers/secretsync_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -146,18 +146,6 @@ func (r *SecretSyncReconciler) Reconcile(ctx context.Context, req ctrl.Request)
return ctrl.Result{}, fmt.Errorf("failed to create client for cluster %s: %w", clusterName, err)
}

ready, err := IsControlPlaneReady(ctx, clusterClient)
if err != nil {
logger.Error(err, "failed to check readiness of cluster", "cluster", cluster.Name)
continue
}

if !ready {
logger.Info("waiting for control plane to be ready", "cluster", cluster.Name)
requeue = true
continue
}

if err := r.syncSecret(ctx, secret, clusterClient, secretSync.Spec.TargetNamespace); err != nil {
logger.Error(err, "failed to sync secret", "cluster", cluster.Name, "secret", secret.Name)
continue
Expand Down
75 changes: 67 additions & 8 deletions controllers/secretsync_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,61 @@ func TestSecretSync(t *testing.T) {
})
}

func TestSecretSyncClusterReadiness(t *testing.T) {
// No control-plane node conditions - no way of knowing that the state.
testCluster, testClusterSecret, testClusterClient := makeTestClusterWithNodeConditions(t, "test")

testSecret := makeTestSecret(types.NamespacedName{
Name: "test-secret",
Namespace: "default",
}, map[string][]byte{"value": []byte("test")})

testSecretSync := makeSecretSync(
"test-secretsync",
testSecret.GetNamespace(),
testSecret.GetName(),
"ns-test",
map[string]string{"environment": "test"},
)

sc, cl := makeTestClientAndScheme(
t, testCluster,
testClusterSecret,
testSecret,
testSecretSync,
)

reconciler := NewSecretSyncReconciler(cl, sc)
reconciler.configParser = func(b []byte) (client.Client, error) {
clusters := map[string]client.Client{
"test": testClusterClient,
}

return clusters[string(b)], nil
}

if _, err := reconciler.Reconcile(context.TODO(), ctrl.Request{NamespacedName: types.NamespacedName{
Name: testSecretSync.GetName(),
Namespace: testSecretSync.GetNamespace(),
}}); err != nil {
t.Fatal(err)
}

var secret v1.Secret
if err := testClusterClient.Get(context.TODO(), client.ObjectKey{Name: "test-secret", Namespace: "ns-test"}, &secret); err != nil {
t.Fatal(err)
}

var secretSync capiv1alpha1.SecretSync
if err := cl.Get(context.TODO(), client.ObjectKeyFromObject(testSecretSync), &secretSync); err != nil {
t.Fatal(err)
}

if _, ok := secretSync.Status.SecretVersions[testCluster.Name]; !ok {
t.Fatalf("secretsync status is not updated")
}
}

func makeSecretSync(name, namespace, secretName, targetNamespace string, selector map[string]string) *capiv1alpha1.SecretSync {
return &capiv1alpha1.SecretSync{
ObjectMeta: metav1.ObjectMeta{
Expand All @@ -161,6 +216,17 @@ func makeSecretSync(name, namespace, secretName, targetNamespace string, selecto
}

func makeReadyTestCluster(t *testing.T, key string) (*clustersv1.GitopsCluster, *v1.Secret, client.Client) {
nodeCondition := corev1.NodeCondition{
Type: "Ready",
Status: "True",
LastHeartbeatTime: metav1.Now(),
LastTransitionTime: metav1.Now(), Reason: "KubeletReady",
Message: "kubelet is posting ready status"}

return makeTestClusterWithNodeConditions(t, key, nodeCondition)
}

func makeTestClusterWithNodeConditions(t *testing.T, key string, conds ...corev1.NodeCondition) (*clustersv1.GitopsCluster, *v1.Secret, client.Client) {
cluster := makeTestCluster(func(c *clustersv1.GitopsCluster) {
c.Name = fmt.Sprintf("cluster-%s", key)
c.Namespace = corev1.NamespaceDefault
Expand All @@ -170,14 +236,7 @@ func makeReadyTestCluster(t *testing.T, key string) (*clustersv1.GitopsCluster,
c.Status.Conditions = append(c.Status.Conditions, makeReadyCondition())
})

nodeCondition := corev1.NodeCondition{
Type: "Ready",
Status: "True",
LastHeartbeatTime: metav1.Now(),
LastTransitionTime: metav1.Now(), Reason: "KubeletReady",
Message: "kubelet is posting ready status"}

readyNode := makeNode(map[string]string{"node-role.kubernetes.io/master": ""}, nodeCondition)
readyNode := makeNode(map[string]string{"node-role.kubernetes.io/master": ""}, conds...)

secret := makeTestSecret(types.NamespacedName{
Name: cluster.GetName() + "-kubeconfig",
Expand Down

0 comments on commit 26d417e

Please sign in to comment.