Skip to content

Commit

Permalink
Removed all existing quorum checks in different controllers
Browse files Browse the repository at this point in the history
Signed-off-by: jubittajohn <[email protected]>

Removed test case related to exisiting quorum checks; Updated the starter.go to remove additional code to call the quorum checker

Signed-off-by: jubittajohn <[email protected]>
  • Loading branch information
jubittajohn committed Jun 26, 2024
1 parent ac57b28 commit fa6e8c3
Show file tree
Hide file tree
Showing 10 changed files with 32 additions and 122 deletions.
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,6 @@ require (

replace (
github.com/gogo/protobuf => github.com/gogo/protobuf v1.3.2
github.com/openshift/library-go => github.com/jubittajohn/library-go v0.0.0-20240621140645-d1afb2e3039d
github.com/openshift/library-go => github.com/jubittajohn/library-go v0.0.0-20240626144247-8d7fddd726eb
vbom.ml/util => github.com/fvbommel/util v0.0.0-20180919145318-efcd4e0f9787
)
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -251,8 +251,8 @@ github.com/json-iterator/go v1.1.12 h1:PV8peI4a0ysnczrg+LtxykD8LfKY9ML6u2jnxaEnr
github.com/json-iterator/go v1.1.12/go.mod h1:e30LSqwooZae/UwlEbR2852Gd8hjQvJoHmT4TnhNGBo=
github.com/jstemmer/go-junit-report v0.0.0-20190106144839-af01ea7f8024/go.mod h1:6v2b51hI/fHJwM22ozAgKL4VKDeJcHhJFhtBdhmNjmU=
github.com/jtolds/gls v4.20.0+incompatible/go.mod h1:QJZ7F/aHp+rZTRtaJ1ow/lLfFfVYBRgL+9YlvaHOwJU=
github.com/jubittajohn/library-go v0.0.0-20240621140645-d1afb2e3039d h1:+pjdApmdTUrRpUtiYQ/blLaU0cEJXBIO8a/cOCLFn4c=
github.com/jubittajohn/library-go v0.0.0-20240621140645-d1afb2e3039d/go.mod h1:PdASVamWinll2BPxiUpXajTwZxV8A1pQbWEsCN1od7I=
github.com/jubittajohn/library-go v0.0.0-20240626144247-8d7fddd726eb h1:d9OacQcqghX3C9E9h45QpWUFPShKR2NzQlFtgnASvp8=
github.com/jubittajohn/library-go v0.0.0-20240626144247-8d7fddd726eb/go.mod h1:PdASVamWinll2BPxiUpXajTwZxV8A1pQbWEsCN1od7I=
github.com/julienschmidt/httprouter v1.2.0/go.mod h1:SYymIcj16QtmaHHD7aYtjjsJG7VTCxuUUipMqKk8s4w=
github.com/julienschmidt/httprouter v1.3.0/go.mod h1:JR6WtHb+2LUe8TCKY3cZOxFyyO8IZAc4RVcycCCAKdM=
github.com/kisielk/errcheck v1.5.0/go.mod h1:pFxgyoBC7bSaBwPgfKdkLd5X25qrDl4LWUI2bnpBCr8=
Expand Down
29 changes: 5 additions & 24 deletions pkg/operator/etcdcertsigner/etcdcertsignercontroller.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,11 @@ import (
"context"
"crypto/x509"
"fmt"
"reflect"
"strconv"
"strings"
"time"

"github.com/openshift/library-go/pkg/crypto"
"github.com/openshift/library-go/pkg/operator/bootstrap"
apierrors "k8s.io/apimachinery/pkg/api/errors"
Expand All @@ -13,10 +18,6 @@ import (
"k8s.io/client-go/tools/cache"
"k8s.io/component-base/metrics"
"k8s.io/klog/v2"
"reflect"
"strconv"
"strings"
"time"

apiannotations "github.com/openshift/api/annotations"
operatorv1 "github.com/openshift/api/operator/v1"
Expand Down Expand Up @@ -330,18 +331,6 @@ func (c *EtcdCertSignerController) syncAllMasterCertificates(
Type: corev1.SecretTypeOpaque,
Data: allCerts,
}

// check the quorum in case the cluster is healthy or not after generating certs, unless we're in force mode
if !forceSkipRollout {
safe, err := c.quorumChecker.IsSafeToUpdateRevision()
if err != nil {
return fmt.Errorf("EtcdCertSignerController can't evaluate whether quorum is safe: %w", err)
}

if !safe {
return fmt.Errorf("skipping EtcdCertSignerController reconciliation due to insufficient quorum")
}
}
_, _, err = resourceapply.ApplySecret(ctx, c.secretClient, recorder, secret)

return err
Expand Down Expand Up @@ -412,14 +401,6 @@ func (c *EtcdCertSignerController) ensureBundles(ctx context.Context,
// the leaf certificates are not regenerated too early.
configMap.Annotations[BundleRolloutRevisionAnnotation] = fmt.Sprintf("%d", currentRevision)

safe, err := c.quorumChecker.IsSafeToUpdateRevision()
if err != nil {
return nil, nil, false, fmt.Errorf("EtcdCertSignerController.ensureBundles can't evaluate whether quorum is safe: %w", err)
}
if !safe {
return nil, nil, false, fmt.Errorf("skipping EtcdCertSignerController.ensureBundles reconciliation due to insufficient quorum")
}

_, rolloutTriggered, err = resourceapply.ApplyConfigMap(ctx, c.configMapClient, recorder, configMap)
if err != nil {
return nil, nil, rolloutTriggered, fmt.Errorf("could not apply bundle configmap: %w", err)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,14 +143,6 @@ func (c *EtcdEndpointsController) syncConfigMap(ctx context.Context, recorder ev

required.Data = endpointAddresses

safe, err := c.quorumChecker.IsSafeToUpdateRevision()
if err != nil {
return fmt.Errorf("EtcdEndpointsController can't evaluate whether quorum is safe: %w", err)
}
if !safe {
return fmt.Errorf("skipping EtcdEndpointsController reconciliation due to insufficient quorum")
}

// Apply endpoint updates
if _, _, err := resourceapply.ApplyConfigMap(ctx, c.configmapClient, recorder, required); err != nil {
return fmt.Errorf("applying configmap update failed :%w", err)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package etcdendpointscontroller

import (
"context"
"fmt"
"testing"

configv1 "github.com/openshift/api/config/v1"
Expand Down Expand Up @@ -269,29 +268,6 @@ func TestBootstrapAnnotationRemoval(t *testing.T) {
}
},
},
{
// The configmap should not update when quorum is critical
name: "ClusterNotUpdateWithMemberChangeViolatingQuorum",
objects: []runtime.Object{
u.BootstrapConfigMap(u.WithBootstrapStatus("complete")),
u.EndpointsConfigMap(
u.WithEndpoint(etcdMembers[0].ID, etcdMembers[0].PeerURLs[0]),
u.WithEndpoint(etcdMembers[1].ID, etcdMembers[1].PeerURLs[0]),
u.WithEndpoint(etcdMembers[2].ID, etcdMembers[2].PeerURLs[0]),
),
},
staticPodStatus: u.StaticPodOperatorStatus(
u.WithLatestRevision(3),
u.WithNodeStatusAtCurrentRevision(3),
u.WithNodeStatusAtCurrentRevision(3),
u.WithNodeStatusAtCurrentRevision(3),
),
expectBootstrap: false,
etcdMembers: []*etcdserverpb.Member{
u.FakeEtcdMemberWithoutServer(0),
},
expectedErr: fmt.Errorf("EtcdEndpointsController can't evaluate whether quorum is safe: %w", fmt.Errorf("etcd cluster has quorum of 1 which is not fault tolerant: [{Member:name:\"etcd-0\" peerURLs:\"https://10.0.0.1:2380\" clientURLs:\"https://10.0.0.1:2907\" Healthy:true Took: Error:<nil>}]")),
},
{
// The configmap should be created without a bootstrap IP because the
// only time the configmap won't already exist is when we've upgraded
Expand Down
16 changes: 1 addition & 15 deletions pkg/operator/starter.go
Original file line number Diff line number Diff line change
Expand Up @@ -301,20 +301,6 @@ func RunOperator(ctx context.Context, controllerContext *controllercmd.Controlle
return !isSNO, precheckSucceeded, err
}

// checks if quorum is valid
quorumSafe := func() (bool, error) {
klog.Info("Hitting quorum valid check")
safe, err := quorumChecker.IsSafeToUpdateRevision()
if !safe {
err = fmt.Errorf("quorum is not safe")
} else if err != nil {
err = fmt.Errorf("unable to evaluate if the quorum is safe: %w", err)
} else {
err = nil
}
return safe, err
}

staticPodControllers, err := staticpod.NewBuilder(operatorClient, kubeClient, kubeInformersForNamespaces, configInformers).
WithEvents(controllerContext.EventRecorder).
WithInstaller([]string{"cluster-etcd-operator", "installer"}).
Expand All @@ -336,7 +322,7 @@ func RunOperator(ctx context.Context, controllerContext *controllercmd.Controlle
guardRolloutPreCheck,
).
WithOperandPodLabelSelector(labels.Set{"etcd": "true"}.AsSelector()).
WithShouldRevisionInstall(quorumSafe).
WithShouldRevisionInstall(quorumChecker.IsSafeToUpdateRevision).
ToControllers()
if err != nil {
return err
Expand Down
18 changes: 0 additions & 18 deletions pkg/operator/targetconfigcontroller/targetconfigcontroller.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,15 +87,6 @@ func NewTargetConfigController(
}

func (c TargetConfigController) sync(ctx context.Context, syncCtx factory.SyncContext) error {
safe, err := c.quorumChecker.IsSafeToUpdateRevision()
if err != nil {
return fmt.Errorf("TargetConfigController can't evaluate whether quorum is safe: %w", err)
}

if !safe {
return fmt.Errorf("skipping TargetConfigController reconciliation due to insufficient quorum")
}

envVars := c.envVarGetter.GetEnvVars()
if len(envVars) == 0 {
return fmt.Errorf("TargetConfigController missing env var values")
Expand All @@ -105,15 +96,6 @@ func (c TargetConfigController) sync(ctx context.Context, syncCtx factory.SyncCo
if err != nil {
return err
}
// check the cluster is healthy or not after get env var, to ensure it is safe to rollout
safe, err = c.quorumChecker.IsSafeToUpdateRevision()
if err != nil {
return fmt.Errorf("TargetConfigController can't evaluate whether quorum is safe: %w", err)
}

if !safe {
return fmt.Errorf("skipping TargetConfigController reconciliation due to insufficient quorum")
}
requeue, err := createTargetConfig(ctx, c, syncCtx.Recorder(), operatorSpec, envVars)
if err != nil {
return err
Expand Down
19 changes: 0 additions & 19 deletions pkg/operator/targetconfigcontroller/targetconfigcontroller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package targetconfigcontroller

import (
"context"
"fmt"
"testing"

configv1 "github.com/openshift/api/config/v1"
Expand Down Expand Up @@ -57,24 +56,6 @@ func TestTargetConfigController(t *testing.T) {
},
etcdMembersEnvVar: "1,2,3",
},
{
name: "Quorum not fault tolerant",
objects: []runtime.Object{
u.BootstrapConfigMap(u.WithBootstrapStatus("complete")),
},
staticPodStatus: u.StaticPodOperatorStatus(
u.WithLatestRevision(3),
u.WithNodeStatusAtCurrentRevision(3),
u.WithNodeStatusAtCurrentRevision(3),
u.WithNodeStatusAtCurrentRevision(3),
),
etcdMembers: []*etcdserverpb.Member{
u.FakeEtcdMemberWithoutServer(0),
u.FakeEtcdMemberWithoutServer(2),
},
etcdMembersEnvVar: "1,3",
expectedErr: fmt.Errorf("TargetConfigController can't evaluate whether quorum is safe: %w", fmt.Errorf("etcd cluster has quorum of 2 which is not fault tolerant: [{Member:name:\"etcd-0\" peerURLs:\"https://10.0.0.1:2380\" clientURLs:\"https://10.0.0.1:2907\" Healthy:true Took: Error:<nil>} {Member:ID:2 name:\"etcd-2\" peerURLs:\"https://10.0.0.3:2380\" clientURLs:\"https://10.0.0.3:2907\" Healthy:true Took: Error:<nil>}]")),
},
{
name: "Quorum not fault tolerant but bootstrapping",
objects: []runtime.Object{
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions vendor/modules.txt
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,7 @@ github.com/openshift/client-go/operator/informers/externalversions/operator/v1
github.com/openshift/client-go/operator/informers/externalversions/operator/v1alpha1
github.com/openshift/client-go/operator/listers/operator/v1
github.com/openshift/client-go/operator/listers/operator/v1alpha1
# github.com/openshift/library-go v0.0.0-20240619140217-e20ca28ddfe7 => github.com/jubittajohn/library-go v0.0.0-20240621140645-d1afb2e3039d
# github.com/openshift/library-go v0.0.0-20240619140217-e20ca28ddfe7 => github.com/jubittajohn/library-go v0.0.0-20240626144247-8d7fddd726eb
## explicit; go 1.22.0
github.com/openshift/library-go/pkg/assets
github.com/openshift/library-go/pkg/authorization/hardcodedauthorizer
Expand Down Expand Up @@ -1542,5 +1542,5 @@ sigs.k8s.io/structured-merge-diff/v4/value
## explicit; go 1.12
sigs.k8s.io/yaml
# github.com/gogo/protobuf => github.com/gogo/protobuf v1.3.2
# github.com/openshift/library-go => github.com/jubittajohn/library-go v0.0.0-20240621140645-d1afb2e3039d
# github.com/openshift/library-go => github.com/jubittajohn/library-go v0.0.0-20240626144247-8d7fddd726eb
# vbom.ml/util => github.com/fvbommel/util v0.0.0-20180919145318-efcd4e0f9787

0 comments on commit fa6e8c3

Please sign in to comment.