Skip to content

Commit

Permalink
Merge pull request #1036 from openshift-cherrypick-robot/cherry-pick-…
Browse files Browse the repository at this point in the history 
…1027-to-release-4.13

[release-4.13] OCPBUGS-10960: skip machine deletion during boostrap
  • Loading branch information
@openshift-merge-robot
openshift-merge-robot committed Mar 29, 2023
2 parents 81a9226 + 2c35b4d commit b633b40
Show file tree
Hide file tree
Showing 9 changed files with 129 additions and 71 deletions.
29 changes: 7 additions & 22 deletions pkg/operator/bootstrapteardown/bootstrap_teardown_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,17 +13,12 @@ import (
"github.com/openshift/library-go/pkg/operator/v1helpers"
"github.com/stretchr/testify/require"
"go.etcd.io/etcd/api/v3/etcdserverpb"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
corev1listers "k8s.io/client-go/listers/core/v1"
"k8s.io/client-go/tools/cache"
)

var (
bootstrapComplete = &corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{Name: "bootstrap", Namespace: "kube-system"},
Data: map[string]string{"status": "complete"},
}
bootstrapComplete = u.BootstrapConfigMap(u.WithBootstrapStatus("complete"))

conditionBootstrapAlreadyRemoved = operatorv1.OperatorCondition{
Type: "EtcdRunningInCluster",
Expand Down Expand Up @@ -60,20 +55,10 @@ var (
Message: "etcd bootstrap member is removed",
}

bootstrapProgressing = &corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{Name: "bootstrap", Namespace: "kube-system"},
Data: map[string]string{"status": "progressing"},
}
bootstrapProgressing = u.BootstrapConfigMap(u.WithBootstrapStatus("progressing"))
)

func TestCanRemoveEtcdBootstrap(t *testing.T) {

defaultEtcdMembers := []*etcdserverpb.Member{
u.FakeEtcdMemberWithoutServer(1),
u.FakeEtcdMemberWithoutServer(2),
u.FakeEtcdMemberWithoutServer(3),
}

tests := map[string]struct {
etcdMembers []*etcdserverpb.Member
clientFakeOpts etcdcli.FakeClientOption
Expand All @@ -83,14 +68,14 @@ func TestCanRemoveEtcdBootstrap(t *testing.T) {
bootstrapId uint64
}{
"default happy path no bootstrap": {
etcdMembers: defaultEtcdMembers,
etcdMembers: u.DefaultEtcdMembers(),
scalingStrategy: ceohelpers.HAScalingStrategy,
safeToRemove: false,
hasBootstrap: false,
bootstrapId: uint64(0),
},
"HA happy path with bootstrap": {
etcdMembers: append(defaultEtcdMembers, u.FakeEtcdBoostrapMember(0)),
etcdMembers: append(u.DefaultEtcdMembers(), u.FakeEtcdBoostrapMember(0)),
scalingStrategy: ceohelpers.HAScalingStrategy,
safeToRemove: true,
hasBootstrap: true,
Expand All @@ -108,7 +93,7 @@ func TestCanRemoveEtcdBootstrap(t *testing.T) {
bootstrapId: uint64(0),
},
"HA with unhealthy member": {
etcdMembers: append(defaultEtcdMembers, u.FakeEtcdBoostrapMember(0)),
etcdMembers: append(u.DefaultEtcdMembers(), u.FakeEtcdBoostrapMember(0)),
clientFakeOpts: etcdcli.WithFakeClusterHealth(&etcdcli.FakeMemberHealth{Unhealthy: 1, Healthy: 3}),
scalingStrategy: ceohelpers.HAScalingStrategy,
safeToRemove: false,
Expand All @@ -129,14 +114,14 @@ func TestCanRemoveEtcdBootstrap(t *testing.T) {
bootstrapId: uint64(0),
},
"DelayedScaling happy path no bootstrap": {
etcdMembers: defaultEtcdMembers,
etcdMembers: u.DefaultEtcdMembers(),
scalingStrategy: ceohelpers.DelayedHAScalingStrategy,
safeToRemove: false,
hasBootstrap: false,
bootstrapId: uint64(0),
},
"DelayedScaling happy path with bootstrap": {
etcdMembers: append(defaultEtcdMembers, u.FakeEtcdBoostrapMember(0)),
etcdMembers: append(u.DefaultEtcdMembers(), u.FakeEtcdBoostrapMember(0)),
scalingStrategy: ceohelpers.DelayedHAScalingStrategy,
safeToRemove: true,
hasBootstrap: true,
Expand Down
21 changes: 17 additions & 4 deletions pkg/operator/ceohelpers/bootstrap.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package ceohelpers
import (
"context"
"fmt"
"github.com/openshift/cluster-etcd-operator/pkg/operator/operatorclient"

configv1listers "github.com/openshift/client-go/config/listers/config/v1"
"github.com/openshift/library-go/pkg/operator/v1helpers"
Expand All @@ -11,7 +12,6 @@ import (
"k8s.io/klog/v2"

"github.com/openshift/cluster-etcd-operator/pkg/etcdcli"
"github.com/openshift/cluster-etcd-operator/pkg/operator/operatorclient"
)

// BootstrapScalingStrategy describes the invariants which will be enforced when
Expand Down Expand Up @@ -60,7 +60,7 @@ const (
DelayedHABootstrapScalingStrategyAnnotation = "openshift.io/delayed-ha-bootstrap"
)

// GetBootstrapScalingStrategy determines the scaling strategy to use.
// GetBootstrapScalingStrategy determines the scaling strategy to use
func GetBootstrapScalingStrategy(staticPodClient v1helpers.StaticPodOperatorClient, namespaceLister corev1listers.NamespaceLister, infraLister configv1listers.InfrastructureLister) (BootstrapScalingStrategy, error) {
var strategy BootstrapScalingStrategy

Expand Down Expand Up @@ -107,7 +107,7 @@ func CheckSafeToScaleCluster(
infraLister configv1listers.InfrastructureLister,
etcdClient etcdcli.EtcdClient) error {

bootstrapComplete, err := IsBootstrapComplete(configmapLister, staticPodClient)
bootstrapComplete, err := IsBootstrapComplete(configmapLister, staticPodClient, etcdClient)
if err != nil {
return fmt.Errorf("CheckSafeToScaleCluster failed to determine bootstrap status: %w", err)
}
Expand Down Expand Up @@ -161,7 +161,7 @@ func CheckSafeToScaleCluster(
}

// IsBootstrapComplete returns true if bootstrap has completed.
func IsBootstrapComplete(configMapClient corev1listers.ConfigMapLister, staticPodClient v1helpers.StaticPodOperatorClient) (bool, error) {
func IsBootstrapComplete(configMapClient corev1listers.ConfigMapLister, staticPodClient v1helpers.StaticPodOperatorClient, etcdClient etcdcli.EtcdClient) (bool, error) {
// do a cheap check to see if the annotation is already gone.
// check to see if bootstrapping is complete
bootstrapFinishedConfigMap, err := configMapClient.ConfigMaps("kube-system").Get("bootstrap")
Expand Down Expand Up @@ -196,5 +196,18 @@ func IsBootstrapComplete(configMapClient corev1listers.ConfigMapLister, staticPo
return false, nil
}
}

// check if etcd-bootstrap member is still present within the etcd cluster membership
membersList, err := etcdClient.MemberList(context.Background())
if err != nil {
return false, fmt.Errorf("IsBootstrapComplete couldn't list the etcd cluster members: %w", err)
}
for _, m := range membersList {
if m.Name == "etcd-bootstrap" {
klog.V(4).Infof("(etcd-bootstrap) member is still present in the etcd cluster membership")
return false, nil
}
}

return true, nil
}
57 changes: 36 additions & 21 deletions pkg/operator/ceohelpers/bootstrap_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -165,36 +165,56 @@ func Test_IsBootstrapComplete(t *testing.T) {
tests := map[string]struct {
bootstrapConfigMap *corev1.ConfigMap
nodes []operatorv1.NodeStatus
etcdMembers []*etcdserverpb.Member
expectComplete bool
expectError error
}{
"bootstrap complete, nodes up to date": {
bootstrapConfigMap: bootstrapComplete,
nodes: twoNodesAtCurrentRevision,
etcdMembers: u.DefaultEtcdMembers(),
expectComplete: true,
expectError: nil,
},
"bootstrap progressing, nodes up to date": {
bootstrapConfigMap: bootstrapProgressing,
nodes: twoNodesAtCurrentRevision,
etcdMembers: u.DefaultEtcdMembers(),
expectComplete: false,
expectError: nil,
},
"bootstrap configmap missing": {
bootstrapConfigMap: nil,
nodes: twoNodesAtCurrentRevision,
etcdMembers: u.DefaultEtcdMembers(),
expectComplete: false,
expectError: nil,
},
"bootstrap complete, no recorded revisions": {
bootstrapConfigMap: bootstrapComplete,
nodes: zeroNodesAtAnyRevision,
etcdMembers: u.DefaultEtcdMembers(),
expectComplete: true,
expectError: nil,
},
"bootstrap complete, node progressing": {
bootstrapConfigMap: bootstrapComplete,
nodes: twoNodesProgressingTowardsCurrentRevision,
etcdMembers: u.DefaultEtcdMembers(),
expectComplete: false,
expectError: nil,
},
"bootstrap complete, etcd-bootstrap removed": {
bootstrapConfigMap: bootstrapComplete,
nodes: twoNodesAtCurrentRevision,
etcdMembers: u.DefaultEtcdMembers(),
expectComplete: true,
expectError: nil,
},
"bootstrap complete, etcd-bootstrap exists": {
bootstrapConfigMap: bootstrapComplete,
nodes: twoNodesAtCurrentRevision,
etcdMembers: append(u.DefaultEtcdMembers(), u.FakeEtcdBoostrapMember(0)),
expectComplete: false,
expectError: nil,
},
Expand All @@ -216,25 +236,20 @@ func Test_IsBootstrapComplete(t *testing.T) {
}
fakeStaticPodClient := v1helpers.NewFakeStaticPodOperatorClient(nil, operatorStatus, nil, nil)

actualComplete, actualErr := IsBootstrapComplete(fakeConfigMapLister, fakeStaticPodClient)

if test.expectComplete != actualComplete {
t.Errorf("expected complete=%v, got %v", test.expectComplete, actualComplete)
}
if test.expectError != actualErr {
t.Errorf("expected error=%v, got %v", test.expectError, actualErr)
fakeEtcdClient, err := etcdcli.NewFakeEtcdClient(test.etcdMembers)
if err != nil {
t.Fatal(err)
}

actualComplete, actualErr := IsBootstrapComplete(fakeConfigMapLister, fakeStaticPodClient, fakeEtcdClient)

assert.Equal(t, test.expectComplete, actualComplete)
assert.Equal(t, test.expectError, actualErr)
})
}
}

func Test_CheckSafeToScaleCluster(t *testing.T) {
defaultEtcdMembers := []*etcdserverpb.Member{
u.FakeEtcdMemberWithoutServer(0),
u.FakeEtcdMemberWithoutServer(1),
u.FakeEtcdMemberWithoutServer(2),
}

tests := map[string]struct {
namespace *corev1.Namespace
bootstrapConfigMap *corev1.ConfigMap
Expand All @@ -250,7 +265,7 @@ func Test_CheckSafeToScaleCluster(t *testing.T) {
bootstrapConfigMap: bootstrapComplete,
operatorConfig: defaultOperatorConfig,
nodes: threeNodesAtCurrentRevision,
etcdMembers: defaultEtcdMembers,
etcdMembers: u.DefaultEtcdMembers(),
infraObj: defaultInfra,
expectError: nil,
},
Expand All @@ -259,7 +274,7 @@ func Test_CheckSafeToScaleCluster(t *testing.T) {
bootstrapConfigMap: bootstrapComplete,
operatorConfig: defaultOperatorConfig,
nodes: twoNodesAtCurrentRevision,
etcdMembers: defaultEtcdMembers,
etcdMembers: u.DefaultEtcdMembers(),
infraObj: defaultInfra,
expectError: fmt.Errorf("CheckSafeToScaleCluster 3 nodes are required, but only 2 are available"),
},
Expand All @@ -268,7 +283,7 @@ func Test_CheckSafeToScaleCluster(t *testing.T) {
bootstrapConfigMap: bootstrapComplete,
operatorConfig: unsupportedOperatorConfig,
nodes: oneNodeAtCurrentRevision,
etcdMembers: defaultEtcdMembers,
etcdMembers: u.DefaultEtcdMembers(),
infraObj: defaultInfra,
expectError: nil,
},
Expand All @@ -277,7 +292,7 @@ func Test_CheckSafeToScaleCluster(t *testing.T) {
bootstrapConfigMap: bootstrapComplete,
operatorConfig: unsupportedOperatorConfig,
nodes: zeroNodesAtAnyRevision,
etcdMembers: defaultEtcdMembers,
etcdMembers: u.DefaultEtcdMembers(),
infraObj: defaultInfra,
expectError: nil,
},
Expand All @@ -286,7 +301,7 @@ func Test_CheckSafeToScaleCluster(t *testing.T) {
bootstrapConfigMap: bootstrapProgressing,
operatorConfig: defaultOperatorConfig,
nodes: twoNodesAtCurrentRevision,
etcdMembers: defaultEtcdMembers,
etcdMembers: u.DefaultEtcdMembers(),
infraObj: defaultInfra,
expectError: nil,
},
Expand All @@ -295,7 +310,7 @@ func Test_CheckSafeToScaleCluster(t *testing.T) {
bootstrapConfigMap: bootstrapProgressing,
operatorConfig: defaultOperatorConfig,
nodes: oneNodeAtCurrentRevision,
etcdMembers: defaultEtcdMembers,
etcdMembers: u.DefaultEtcdMembers(),
infraObj: defaultInfra,
expectError: nil,
},
Expand All @@ -304,7 +319,7 @@ func Test_CheckSafeToScaleCluster(t *testing.T) {
bootstrapConfigMap: bootstrapComplete,
operatorConfig: defaultOperatorConfig,
nodes: threeNodesAtCurrentRevision,
etcdMembers: defaultEtcdMembers,
etcdMembers: u.DefaultEtcdMembers(),
infraObj: defaultInfra,
expectError: nil,
},
Expand All @@ -313,7 +328,7 @@ func Test_CheckSafeToScaleCluster(t *testing.T) {
bootstrapConfigMap: bootstrapComplete,
operatorConfig: defaultOperatorConfig,
nodes: twoNodesAtCurrentRevision,
etcdMembers: defaultEtcdMembers,
etcdMembers: u.DefaultEtcdMembers(),
infraObj: defaultInfra,
expectError: fmt.Errorf("CheckSafeToScaleCluster 3 nodes are required, but only 2 are available"),
},
Expand Down
26 changes: 14 additions & 12 deletions pkg/operator/ceohelpers/common_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,20 @@ import (
"github.com/openshift/library-go/pkg/operator/v1helpers"
)

var (
withWellKnownReplicasCountSet = `
{
"controlPlane": {"replicas": 3}
}
`

withReplicasCountSetInUnsupportedConfig = `
{
"controlPlane": {"replicas": 7}
}
`
)

func TestReadDesiredControlPlaneReplicaCount(t *testing.T) {
scenarios := []struct {
name string
Expand Down Expand Up @@ -61,15 +75,3 @@ func TestReadDesiredControlPlaneReplicaCount(t *testing.T) {
})
}
}

var withWellKnownReplicasCountSet = `
{
"controlPlane": {"replicas": 3}
}
`

var withReplicasCountSetInUnsupportedConfig = `
{
"controlPlane": {"replicas": 7}
}
`
Loading

0 comments on commit b633b40

Please sign in to comment.