Update reconciler logic for server,peer certificate

Signed-off-by: Arka Saha <[email protected]>

Author: Arka Saha

config/samples/operator_v1alpha1_etcdcluster.yaml

@@ -15,4 +15,4 @@ spec:
         commonName: "etcd-operator-system"
         validityDuration: "90h"
         issuerName: "selfsigned"
-        issuerKind: "ClusterIssuer"
+        issuerKind: "ClusterIssuer"

internal/controller/etcdcluster_controller.go

@@ -87,13 +87,13 @@ func (r *EtcdClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 
 	// Create Client Certificate for etcd-operator to communicate with the etcdCluster
 	if etcdCluster.Spec.TLS != nil {
-		clientCertErr := r.checkClientCertificate(etcdCluster, ctx)
+		clientCertErr := createClientCertificate(etcdCluster, ctx, r.Client)
 		if clientCertErr != nil {
 			logger.Error(clientCertErr, "Failed to create Client Certificate.")
 		}
 	} else {
 		// TODO: instead of logging error, set default autoConfig
-		logger.Error(fmt.Errorf("missing TLS config for %s", etcdCluster.Name), "certificates cannot be created")
+		logger.Error(nil, fmt.Sprintf("missing TLS config for %s", etcdCluster.Name))
 	}
 
 	logger.Info("Reconciling EtcdCluster", "spec", etcdCluster.Spec)
@@ -105,7 +105,7 @@ func (r *EtcdClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 			logger.Info("Creating StatefulSet with 0 replica", "expectedSize", etcdCluster.Spec.Size)
 			// Create a new StatefulSet
 
-			sts, err = reconcileStatefulSet(ctx, logger, etcdCluster, r.Client, 0, r.Scheme)
+			sts, err = reconcileStatefulSet(ctx, logger, etcdCluster, r.Client, 0, r.Scheme, true)
 			if err != nil {
 				return ctrl.Result{}, err
 			}
@@ -118,17 +118,6 @@ func (r *EtcdClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 		}
 	}
 
-	// Create Server and Peer Certificate for etcd-operator to communicate within the members
-	if etcdCluster.Spec.TLS != nil {
-		createServerPeerCertErr := r.checkServerPeerCertificate(etcdCluster, sts, ctx)
-		if createServerPeerCertErr != nil {
-			logger.Error(createServerPeerCertErr, "Error creating Server or Peer Certificate")
-		}
-	} else {
-		// TODO: instead of logging error, set default autoConfig
-		logger.Error(fmt.Errorf("missing TLS config for %s", etcdCluster.Name), "certificates cannot be created")
-	}
-
 	// If the Statefulsets is not controlled by this EtcdCluster resource, we should log
 	// a warning to the event recorder and return error msg.
 	err = checkStatefulSetControlledByEtcdOperator(etcdCluster, sts)
@@ -141,7 +130,7 @@ func (r *EtcdClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 	if sts.Spec.Replicas != nil && *sts.Spec.Replicas == 0 {
 		logger.Info("StatefulSet has 0 replicas. Trying to create a new cluster with 1 member")
 
-		sts, err = reconcileStatefulSet(ctx, logger, etcdCluster, r.Client, 1, r.Scheme)
+		sts, err = reconcileStatefulSet(ctx, logger, etcdCluster, r.Client, 1, r.Scheme, true)
 		if err != nil {
 			return ctrl.Result{}, err
 		}
@@ -171,15 +160,15 @@ func (r *EtcdClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 			logger.Info("An etcd member was added into the cluster, but the StatefulSet hasn't scaled out yet")
 			newReplicaCount := targetReplica + 1
 			logger.Info("Increasing StatefulSet replicas to match the etcd cluster member count", "oldReplicaCount", targetReplica, "newReplicaCount", newReplicaCount)
-			_, err = reconcileStatefulSet(ctx, logger, etcdCluster, r.Client, newReplicaCount, r.Scheme)
+			_, err = reconcileStatefulSet(ctx, logger, etcdCluster, r.Client, newReplicaCount, r.Scheme, true)
 			if err != nil {
 				return ctrl.Result{}, err
 			}
 		} else {
 			logger.Info("An etcd member was removed from the cluster, but the StatefulSet hasn't scaled in yet")
 			newReplicaCount := targetReplica - 1
 			logger.Info("Decreasing StatefulSet replicas to remove the unneeded Pod.", "oldReplicaCount", targetReplica, "newReplicaCount", newReplicaCount)
-			_, err = reconcileStatefulSet(ctx, logger, etcdCluster, r.Client, newReplicaCount, r.Scheme)
+			_, err = reconcileStatefulSet(ctx, logger, etcdCluster, r.Client, newReplicaCount, r.Scheme, false)
 			if err != nil {
 				return ctrl.Result{}, err
 			}
@@ -244,6 +233,11 @@ func (r *EtcdClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 		}
 
 		logger.Info("Learner member added successfully", "peerURLs", peerURL)
+
+		sts, err = reconcileStatefulSet(ctx, logger, etcdCluster, r.Client, targetReplica, r.Scheme, true)
+		if err != nil {
+			return ctrl.Result{}, err
+		}
 	} else {
 		// scale in
 		targetReplica--
@@ -256,11 +250,11 @@ func (r *EtcdClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 		if err := etcdutils.RemoveMember(eps, memberID); err != nil {
 			return ctrl.Result{}, err
 		}
-	}
 
-	sts, err = reconcileStatefulSet(ctx, logger, etcdCluster, r.Client, targetReplica, r.Scheme)
-	if err != nil {
-		return ctrl.Result{}, err
+		sts, err = reconcileStatefulSet(ctx, logger, etcdCluster, r.Client, targetReplica, r.Scheme, false)
+		if err != nil {
+			return ctrl.Result{}, err
+		}
 	}
 
 	allMembersHealthy, err := areAllMembersHealthy(sts, logger)

internal/controller/utils.go

@@ -65,7 +65,7 @@ func prepareOwnerReference(ec *ecv1alpha1.EtcdCluster, scheme *runtime.Scheme) (
 	return owners, nil
 }
 
-func reconcileStatefulSet(ctx context.Context, logger logr.Logger, ec *ecv1alpha1.EtcdCluster, c client.Client, replicas int32, scheme *runtime.Scheme) (*appsv1.StatefulSet, error) {
+func reconcileStatefulSet(ctx context.Context, logger logr.Logger, ec *ecv1alpha1.EtcdCluster, c client.Client, replicas int32, scheme *runtime.Scheme, addMember bool) (*appsv1.StatefulSet, error) {
 
 	// prepare/update configmap for StatefulSet
 	err := applyEtcdClusterState(ctx, ec, int(replicas), c, scheme, logger)
@@ -85,6 +85,26 @@ func reconcileStatefulSet(ctx context.Context, logger logr.Logger, ec *ecv1alpha
 		return nil, err
 	}
 
+	// Add or remove server and peer certificate
+	if addMember {
+		if replicas > 0 {
+			if ec.Spec.TLS != nil {
+				createServerPeerCertErr := createServerPeerCertificate(ec, replicas, ctx, c)
+				if createServerPeerCertErr != nil {
+					logger.Error(createServerPeerCertErr, "Error creating Server or Peer Certificate")
+				}
+			} else {
+				// TODO: instead of logging error, set default autoConfig
+				logger.Error(nil, fmt.Sprintf("missing TLS config for %s", ec.Name))
+			}
+		}
+	} else {
+		deleteServerPeerCertErr := deleteServerPeerCertificate(ec, replicas, ctx, c)
+		if deleteServerPeerCertErr != nil {
+			logger.Error(deleteServerPeerCertErr, "Error deleting Server or Peer Certificate")
+		}
+	}
+
 	// Return latest Stateful set. (This is to ensure that we return the latest statefulset for next operation to act on)
 	return getStatefulSet(ctx, c, ec.Name, ec.Namespace)
 }
@@ -528,15 +548,6 @@ func healthCheck(sts *appsv1.StatefulSet, lg klog.Logger) (*clientv3.MemberListR
 	return memberlistResp, healthInfos, nil
 }
 
-func (r *EtcdClusterReconciler) getStatefulSetPods(sts *appsv1.StatefulSet, ctx context.Context) (*corev1.PodList, error) {
-	podList := corev1.PodList{}
-	err := r.Client.List(ctx, &podList, client.InNamespace(sts.Namespace), client.MatchingLabels(sts.Spec.Selector.MatchLabels))
-	if err != nil {
-		return nil, err
-	}
-	return &podList, nil
-}
-
 func createCMCertificateConfig(ec *ecv1alpha1.ProviderCertManagerConfig) *certInterface.Config {
 	duration, err := time.ParseDuration(ec.ValidityDuration)
 	if err != nil {
@@ -564,8 +575,8 @@ func createAutoCertificateConfig(ec *ecv1alpha1.ProviderAutoConfig) *certInterfa
 	return config
 }
 
-func (r *EtcdClusterReconciler) createCertificate(ec *ecv1alpha1.EtcdCluster, ctx context.Context, certName string) error {
-	cert, certErr := certificate.NewProvider(certificate.ProviderType(ec.Spec.TLS.Provider), r.Client)
+func createCertificate(ec *ecv1alpha1.EtcdCluster, ctx context.Context, c client.Client, certName string) error {
+	cert, certErr := certificate.NewProvider(certificate.ProviderType(ec.Spec.TLS.Provider), c)
 	if certErr != nil {
 		// TODO: instead of error, set default autoConfig
 		return certErr
@@ -608,24 +619,51 @@ func (r *EtcdClusterReconciler) createCertificate(ec *ecv1alpha1.EtcdCluster, ct
 	return nil
 }
 
-func (r *EtcdClusterReconciler) checkClientCertificate(ec *ecv1alpha1.EtcdCluster, ctx context.Context) error {
+func deleteCertificate(ec *ecv1alpha1.EtcdCluster, ctx context.Context, c client.Client, certName string) error {
+	cert, certErr := certificate.NewProvider(certificate.ProviderType(ec.Spec.TLS.Provider), c)
+	if certErr != nil {
+		// TODO: instead of error, set default autoConfig
+		return certErr
+	}
+	deleteCertErr := cert.DeleteCertificateSecret(ctx, certName, ec.Namespace)
+	if deleteCertErr != nil {
+		log.Printf("Error deleting certificate with name %s: %s", certName, deleteCertErr)
+		return deleteCertErr
+	}
+	log.Printf("Successfully deleted certificate with name %s", certName)
+	return nil
+}
+
+func createClientCertificate(ec *ecv1alpha1.EtcdCluster, ctx context.Context, c client.Client) error {
 	certName := fmt.Sprintf("%s-%s-tls", ec.Name, "client")
-	createClientCertErr := r.createCertificate(ec, ctx, certName)
+	createClientCertErr := createCertificate(ec, ctx, c, certName)
 	return createClientCertErr
 }
 
-func (r *EtcdClusterReconciler) checkServerPeerCertificate(ec *ecv1alpha1.EtcdCluster, sts *appsv1.StatefulSet, ctx context.Context) error {
-	podList, podListErr := r.getStatefulSetPods(sts, ctx)
-	if podListErr != nil {
-		return podListErr
-	}
-	for _, pod := range podList.Items {
-		serverCertName := fmt.Sprintf("%s-%s-tls", pod.Name, "server")
-		peerCertName := fmt.Sprintf("%s-%s-tls", pod.Name, "peer")
-		createServerCertErr := r.createCertificate(ec, ctx, serverCertName)
-		log.Println(createServerCertErr)
-		createPeerCertErr := r.createCertificate(ec, ctx, peerCertName)
-		log.Println(createPeerCertErr)
+func createServerPeerCertificate(ec *ecv1alpha1.EtcdCluster, replicas int32, ctx context.Context, c client.Client) error {
+	serverCertName := fmt.Sprintf("%s-%s-%s-tls", ec.Name, string(replicas-1), "server")
+	peerCertName := fmt.Sprintf("%s-%s-%s-tls", ec.Name, string(replicas-1), "peer")
+	createServerCertErr := createCertificate(ec, ctx, c, serverCertName)
+	if createServerCertErr != nil {
+		return createServerCertErr
+	}
+	createPeerCertErr := createCertificate(ec, ctx, c, peerCertName)
+	if createPeerCertErr != nil {
+		return createPeerCertErr
+	}
+	return nil
+}
+
+func deleteServerPeerCertificate(ec *ecv1alpha1.EtcdCluster, replicas int32, ctx context.Context, c client.Client) error {
+	serverCertName := fmt.Sprintf("%s-%s-%s-tls", ec.Name, string(replicas), "server")
+	peerCertName := fmt.Sprintf("%s-%s-%s-tls", ec.Name, string(replicas), "peer")
+	deleteServerCertErr := deleteCertificate(ec, ctx, c, serverCertName)
+	if deleteServerCertErr != nil {
+		return deleteServerCertErr
+	}
+	deletePeerCertErr := deleteCertificate(ec, ctx, c, peerCertName)
+	if deletePeerCertErr != nil {
+		return deletePeerCertErr
 	}
 	return nil
 }

internal/controller/utils_test.go

@@ -16,10 +16,11 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 	"sigs.k8s.io/controller-runtime/pkg/log"
 
-	ecv1alpha1 "go.etcd.io/etcd-operator/api/v1alpha1"
-	"go.etcd.io/etcd-operator/internal/etcdutils"
 	"go.etcd.io/etcd/api/v3/etcdserverpb"
 	clientv3 "go.etcd.io/etcd/client/v3"
+
+	ecv1alpha1 "go.etcd.io/etcd-operator/api/v1alpha1"
+	"go.etcd.io/etcd-operator/internal/etcdutils"
 )
 
 func TestPrepareOwnerReference(t *testing.T) {
@@ -67,7 +68,7 @@ func TestReconcileStatefulSet(t *testing.T) {
 		},
 	}
 
-	_, _ = reconcileStatefulSet(t.Context(), logger, ec, fakeClient, 3, scheme)
+	_, _ = reconcileStatefulSet(t.Context(), logger, ec, fakeClient, 3, scheme, true)
 
 	sts := &appsv1.StatefulSet{}
 	err := fakeClient.Get(t.Context(), client.ObjectKey{Name: "test-etcd", Namespace: "default"}, sts)