Revert "ETCD-579: store revision at which signers are rotated"

This reverts commit eb37f65.
openshift · Apr 12, 2024 · 45c319a · 45c319a
1 parent e0d9e7c
commit 45c319a
Show file tree

Hide file tree

Showing 9 changed files with 58 additions and 544 deletions.
diff --git a/pkg/cmd/render/certs.go b/pkg/cmd/render/certs.go
@@ -36,30 +36,26 @@ func createCertSecrets(nodes []*corev1.Node) ([]corev1.Secret, []corev1.ConfigMa
 			ManagementState: operatorv1.Managed,
 		},
 	}, &operatorv1.StaticPodOperatorStatus{
-		LatestAvailableRevision: 1,
-		OperatorStatus:          operatorv1.OperatorStatus{Conditions: []operatorv1.OperatorCondition{}},
-		NodeStatuses:            []operatorv1.NodeStatus{{CurrentRevision: 1, TargetRevision: 1}},
+		OperatorStatus: operatorv1.OperatorStatus{Conditions: []operatorv1.OperatorCondition{}},
+		NodeStatuses:   []operatorv1.NodeStatus{},
 	}, nil, nil)
 
 	kubeInformers := v1helpers.NewKubeInformersForNamespaces(fakeKubeClient, "", "kube-system",
 		operatorclient.TargetNamespace, operatorclient.OperatorNamespace, operatorclient.GlobalUserSpecifiedConfigNamespace)
-	secretInformer := kubeInformers.InformersFor(operatorclient.GlobalUserSpecifiedConfigNamespace).Core().V1().Secrets()
-	secretClient := fakeKubeClient.CoreV1()
-	secretLister := &tlshelpers.SecretClientLister{
-		SecretClient: secretClient.Secrets(operatorclient.GlobalUserSpecifiedConfigNamespace),
-		Namespace:    operatorclient.GlobalUserSpecifiedConfigNamespace,
-	}
+	secretInformer := kubeInformers.InformersFor(operatorclient.TargetNamespace).Core().V1().Secrets()
+	secretLister := secretInformer.Lister()
+	secretClient := v1helpers.CachedSecretGetter(fakeKubeClient.CoreV1(), kubeInformers)
 
 	recorder := events.NewInMemoryRecorder("etcd")
 	// create openshift-config signers first, they will remain in openshift-config and are needed for the controller sync loop to function
 	// TODO(thomas): once the rotation process is in place, we can remove that special case
 	etcdSignerCert := tlshelpers.CreateBootstrapSignerCert(secretInformer, secretLister, secretClient, recorder)
-	_, _, err := etcdSignerCert.EnsureSigningCertKeyPair(context.Background())
+	_, err := etcdSignerCert.EnsureSigningCertKeyPair(context.Background())
 	if err != nil {
 		return nil, nil, fmt.Errorf("could not create etcd signer certificate: %w", err)
 	}
 	metricsSignerCert := tlshelpers.CreateBootstrapMetricsSignerCert(secretInformer, secretLister, secretClient, recorder)
-	_, _, err = metricsSignerCert.EnsureSigningCertKeyPair(context.Background())
+	_, err = metricsSignerCert.EnsureSigningCertKeyPair(context.Background())
 	if err != nil {
 		return nil, nil, fmt.Errorf("could not create etcd metrics signer certificate: %w", err)
 	}

diff --git a/pkg/cmd/render/certs_test.go b/pkg/cmd/render/certs_test.go
@@ -133,7 +133,6 @@ func assertBundleCorrectness(t *testing.T, secrets []corev1.Secret, bundles []co
 			return bytes.Compare(bundleCerts[i].Raw, bundleCerts[j].Raw) < 0
 		})
 
-		require.Equal(t, len(signers), len(bundleCerts))
 		require.Equal(t, signers, bundleCerts)
 	}
 }

diff --git a/pkg/operator/ceohelpers/common.go b/pkg/operator/ceohelpers/common.go
@@ -4,7 +4,6 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
-	operatorv1 "github.com/openshift/api/operator/v1"
 	"github.com/openshift/cluster-etcd-operator/pkg/dnshelpers"
 	"github.com/openshift/cluster-etcd-operator/pkg/etcdcli"
 	"net"
@@ -30,8 +29,6 @@ const MachineDeletionHookName = "EtcdQuorumOperator"
 // MachineDeletionHookOwner holds an owner of the Machine Deletion Hook
 const MachineDeletionHookOwner = "clusteroperator/etcd"
 
-var RevisionRolloutInProgressErr = fmt.Errorf("revision rollout in progress, can't establish current revision")
-
 // ReadDesiredControlPlaneReplicasCount reads the current Control Plane replica count
 func ReadDesiredControlPlaneReplicasCount(operatorClient v1helpers.StaticPodOperatorClient) (int, error) {
 	operatorSpec, _, _, err := operatorClient.GetStaticPodOperatorState()
@@ -200,38 +197,3 @@ func VotingMemberIPListSet(ctx context.Context, cli etcdcli.EtcdClient) (sets.St
 
 	return currentVotingMemberIPListSet, nil
 }
-
-// RevisionRolloutInProgress will return true if any node status reports its target revision is different from the current revision and the latest known revision.
-func RevisionRolloutInProgress(status operatorv1.StaticPodOperatorStatus) bool {
-	latestRevision := status.LatestAvailableRevision
-	for _, nodeStatus := range status.NodeStatuses {
-		if (nodeStatus.TargetRevision > 0 && nodeStatus.CurrentRevision != nodeStatus.TargetRevision) ||
-			nodeStatus.CurrentRevision != latestRevision {
-			return true
-		}
-	}
-
-	return false
-}
-
-// CurrentRevision will only return the current revision if no revision rollout is in progress and all revisions across nodes
-// are the exact same. Otherwise, an error will be returned.
-func CurrentRevision(status operatorv1.StaticPodOperatorStatus) (int32, error) {
-	if RevisionRolloutInProgress(status) {
-		return 0, RevisionRolloutInProgressErr
-	}
-
-	if len(status.NodeStatuses) == 0 {
-		return 0, fmt.Errorf("no node status")
-	}
-
-	latestRevision := status.LatestAvailableRevision
-	for _, nodeStatus := range status.NodeStatuses {
-		if latestRevision != nodeStatus.CurrentRevision {
-			return 0, fmt.Errorf("node [%s] is not on latest revision yet: %d vs latest revision %d",
-				nodeStatus.NodeName, nodeStatus.CurrentRevision, latestRevision)
-		}
-	}
-
-	return latestRevision, nil
-}
diff --git a/pkg/operator/ceohelpers/common_test.go b/pkg/operator/ceohelpers/common_test.go
@@ -1,8 +1,6 @@
 package ceohelpers
 
 import (
-	"errors"
-	"github.com/stretchr/testify/require"
 	"testing"
 
 	"k8s.io/apimachinery/pkg/runtime"
@@ -77,152 +75,3 @@ func TestReadDesiredControlPlaneReplicaCount(t *testing.T) {
 		})
 	}
 }
-
-func TestRevisionRolloutInProgress(t *testing.T) {
-	scenarios := []struct {
-		name     string
-		status   operatorv1.StaticPodOperatorStatus
-		expected bool
-	}{
-		{
-			name: "revs equal single node",
-			status: operatorv1.StaticPodOperatorStatus{
-				LatestAvailableRevision: 1,
-				NodeStatuses: []operatorv1.NodeStatus{
-					{NodeName: "node-1", CurrentRevision: 1, TargetRevision: 1},
-				},
-			},
-			expected: false,
-		},
-		{
-			name: "revs not equal single node",
-			status: operatorv1.StaticPodOperatorStatus{
-				LatestAvailableRevision: 3,
-				NodeStatuses: []operatorv1.NodeStatus{
-					{NodeName: "node-1", CurrentRevision: 1, TargetRevision: 3},
-				},
-			},
-			expected: true,
-		},
-		{
-			name: "revs not equal multi node",
-			status: operatorv1.StaticPodOperatorStatus{
-				LatestAvailableRevision: 3,
-				NodeStatuses: []operatorv1.NodeStatus{
-					{NodeName: "node-1", CurrentRevision: 3, TargetRevision: 3},
-					{NodeName: "node-2", CurrentRevision: 1, TargetRevision: 3},
-				},
-			},
-			expected: true,
-		},
-		{
-			name: "revs equal multi node",
-			status: operatorv1.StaticPodOperatorStatus{
-				LatestAvailableRevision: 3,
-				NodeStatuses: []operatorv1.NodeStatus{
-					{NodeName: "node-1", CurrentRevision: 3, TargetRevision: 3},
-					{NodeName: "node-2", CurrentRevision: 3, TargetRevision: 3},
-				},
-			},
-			expected: false,
-		},
-	}
-
-	for _, scenario := range scenarios {
-		t.Run(scenario.name, func(t *testing.T) {
-			require.Equal(t, scenario.expected, RevisionRolloutInProgress(scenario.status))
-		})
-	}
-}
-
-func TestCurrentRevision(t *testing.T) {
-	scenarios := []struct {
-		name        string
-		status      operatorv1.StaticPodOperatorStatus
-		expected    int32
-		expectedErr error
-	}{
-		{
-			name: "revs equal single node",
-			status: operatorv1.StaticPodOperatorStatus{
-				NodeStatuses: []operatorv1.NodeStatus{},
-			},
-			expectedErr: errors.New("no node status"),
-		},
-		{
-			name: "revs equal single node",
-			status: operatorv1.StaticPodOperatorStatus{
-				LatestAvailableRevision: 22,
-				NodeStatuses: []operatorv1.NodeStatus{
-					{NodeName: "node-1", CurrentRevision: 22, TargetRevision: 22},
-				},
-			},
-			expected: 22,
-		},
-		{
-			name: "revs not equal single node",
-			status: operatorv1.StaticPodOperatorStatus{
-				LatestAvailableRevision: 3,
-				NodeStatuses: []operatorv1.NodeStatus{
-					{NodeName: "node-1", CurrentRevision: 1, TargetRevision: 3},
-				},
-			},
-			expected:    0,
-			expectedErr: RevisionRolloutInProgressErr,
-		},
-		{
-			name: "target revs not equal multi node",
-			status: operatorv1.StaticPodOperatorStatus{
-				LatestAvailableRevision: 3,
-				NodeStatuses: []operatorv1.NodeStatus{
-					{NodeName: "node-1", CurrentRevision: 3, TargetRevision: 3},
-					{NodeName: "node-2", CurrentRevision: 1, TargetRevision: 3},
-				},
-			},
-			expected:    0,
-			expectedErr: RevisionRolloutInProgressErr,
-		},
-		{
-			name: "revs equal multi node",
-			status: operatorv1.StaticPodOperatorStatus{
-				LatestAvailableRevision: 3,
-				NodeStatuses: []operatorv1.NodeStatus{
-					{NodeName: "node-1", CurrentRevision: 3, TargetRevision: 0},
-					{NodeName: "node-2", CurrentRevision: 3, TargetRevision: 0},
-				},
-			},
-			expected: 3,
-		},
-		{
-			name: "revs differ multi node",
-			status: operatorv1.StaticPodOperatorStatus{
-				LatestAvailableRevision: 3,
-				NodeStatuses: []operatorv1.NodeStatus{
-					{NodeName: "node-1", CurrentRevision: 3, TargetRevision: 3},
-					{NodeName: "node-2", CurrentRevision: 2, TargetRevision: 2},
-				},
-			},
-			expected:    0,
-			expectedErr: errors.New("revision rollout in progress, can't establish current revision"),
-		},
-		{
-			name: "latest rev far ahead",
-			status: operatorv1.StaticPodOperatorStatus{
-				LatestAvailableRevision: 25,
-				NodeStatuses: []operatorv1.NodeStatus{
-					{NodeName: "node-1", CurrentRevision: 3, TargetRevision: 3},
-				},
-			},
-			expected:    0,
-			expectedErr: errors.New("revision rollout in progress, can't establish current revision"),
-		},
-	}
-
-	for _, scenario := range scenarios {
-		t.Run(scenario.name, func(t *testing.T) {
-			revision, err := CurrentRevision(scenario.status)
-			require.Equal(t, scenario.expectedErr, err)
-			require.Equal(t, scenario.expected, revision)
-		})
-	}
-}