Skip to content

Commit

Permalink
OCPBUGS-42809: always ensure quorum during bootstrap
Browse files Browse the repository at this point in the history
  • Loading branch information
tjungblu committed Nov 28, 2024
1 parent 47b36cf commit eefee25
Show file tree
Hide file tree
Showing 5 changed files with 5 additions and 42 deletions.
10 changes: 0 additions & 10 deletions pkg/operator/ceohelpers/bootstrap.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,21 +101,11 @@ func GetBootstrapScalingStrategy(staticPodClient v1helpers.StaticPodOperatorClie
// the etcd cluster based on the scaling strategy in use, and otherwise will return
// an error explaining why it's unsafe to scale.
func CheckSafeToScaleCluster(
configmapLister corev1listers.ConfigMapLister,
staticPodClient v1helpers.StaticPodOperatorClient,
namespaceLister corev1listers.NamespaceLister,
infraLister configv1listers.InfrastructureLister,
etcdClient etcdcli.AllMemberLister) error {

bootstrapComplete, err := IsBootstrapComplete(configmapLister, etcdClient)
if err != nil {
return fmt.Errorf("CheckSafeToScaleCluster failed to determine bootstrap status: %w", err)
}
// while bootstrapping, scaling should be considered safe always
if !bootstrapComplete {
return nil
}

revisionStable, err := IsRevisionStable(staticPodClient)
if err != nil {
return fmt.Errorf("CheckSafeToScaleCluster failed to determine stability of revisions: %w", err)
Expand Down
2 changes: 0 additions & 2 deletions pkg/operator/ceohelpers/bootstrap_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,6 @@ func Test_CheckSafeToScaleCluster(t *testing.T) {
t.Fatal(err)
}
}
fakeConfigMapLister := corev1listers.NewConfigMapLister(configmapIndexer)

operatorStatus := &operatorv1.StaticPodOperatorStatus{
OperatorStatus: operatorv1.OperatorStatus{LatestAvailableRevision: 1},
Expand All @@ -379,7 +378,6 @@ func Test_CheckSafeToScaleCluster(t *testing.T) {
}

actualErr := CheckSafeToScaleCluster(
fakeConfigMapLister,
fakeStaticPodClient,
fakeNamespaceMapLister,
fakeInfraStructure,
Expand Down
5 changes: 1 addition & 4 deletions pkg/operator/ceohelpers/qourum_check.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,14 @@ func (c *AlwaysSafeQuorumChecker) IsSafeToUpdateRevision() (bool, error) {

// QuorumCheck is just a convenience struct around bootstrap.go
type QuorumCheck struct {
configMapLister corev1listers.ConfigMapLister
namespaceLister corev1listers.NamespaceLister
infraLister configv1listers.InfrastructureLister
operatorClient v1helpers.StaticPodOperatorClient
etcdClient etcdcli.AllMemberLister
}

func (c *QuorumCheck) IsSafeToUpdateRevision() (bool, error) {
err := CheckSafeToScaleCluster(c.configMapLister, c.operatorClient, c.namespaceLister, c.infraLister, c.etcdClient)
err := CheckSafeToScaleCluster(c.operatorClient, c.namespaceLister, c.infraLister, c.etcdClient)
if err != nil {
return false, err
}
Expand All @@ -43,14 +42,12 @@ func (c *QuorumCheck) IsSafeToUpdateRevision() (bool, error) {
}

func NewQuorumChecker(
configMapLister corev1listers.ConfigMapLister,
namespaceLister corev1listers.NamespaceLister,
infraLister configv1listers.InfrastructureLister,
operatorClient v1helpers.StaticPodOperatorClient,
etcdClient etcdcli.AllMemberLister,
) QuorumChecker {
c := &QuorumCheck{
configMapLister,
namespaceLister,
infraLister,
operatorClient,
Expand Down
29 changes: 4 additions & 25 deletions pkg/operator/ceohelpers/qourum_check_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,8 @@ func TestQuorumCheck_IsSafeToUpdateRevision(t *testing.T) {
expectedErr error
}{
{
name: "HappyPath",
objects: []runtime.Object{
u.BootstrapConfigMap(u.WithBootstrapStatus("complete")),
},
name: "HappyPath",
objects: []runtime.Object{},
staticPodStatus: u.StaticPodOperatorStatus(
u.WithLatestRevision(3),
u.WithNodeStatusAtCurrentRevision(3),
Expand All @@ -70,26 +68,8 @@ func TestQuorumCheck_IsSafeToUpdateRevision(t *testing.T) {
safe: true,
},
{
name: "Incomplete Quorum during bootstrap",
objects: []runtime.Object{
u.BootstrapConfigMap(u.WithBootstrapStatus("in progress")),
},
staticPodStatus: u.StaticPodOperatorStatus(
u.WithLatestRevision(3),
u.WithNodeStatusAtCurrentRevision(3),
u.WithNodeStatusAtCurrentRevision(3),
u.WithNodeStatusAtCurrentRevision(3),
),
etcdMembers: []*etcdserverpb.Member{
u.FakeEtcdMemberWithoutServer(0),
},
safe: true,
},
{
name: "Incomplete Quorum",
objects: []runtime.Object{
u.BootstrapConfigMap(u.WithBootstrapStatus("complete")),
},
name: "Incomplete Quorum",
objects: []runtime.Object{},
staticPodStatus: u.StaticPodOperatorStatus(
u.WithLatestRevision(3),
u.WithNodeStatusAtCurrentRevision(3),
Expand Down Expand Up @@ -131,7 +111,6 @@ func TestQuorumCheck_IsSafeToUpdateRevision(t *testing.T) {
}

quorumChecker := NewQuorumChecker(
corev1listers.NewConfigMapLister(indexer),
corev1listers.NewNamespaceLister(indexer),
configv1listers.NewInfrastructureLister(indexer),
fakeOperatorClient,
Expand Down
1 change: 0 additions & 1 deletion pkg/operator/starter.go
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,6 @@ func RunOperator(ctx context.Context, controllerContext *controllercmd.Controlle
)

quorumChecker := ceohelpers.NewQuorumChecker(
kubeInformersForNamespaces.ConfigMapLister(),
kubeInformersForNamespaces.InformersFor(operatorclient.TargetNamespace).Core().V1().Namespaces().Lister(),
configInformers.Config().V1().Infrastructures().Lister(),
operatorClient,
Expand Down

0 comments on commit eefee25

Please sign in to comment.