Graduate feature UseEtcdWrapper to GA, locked to true (#936)

gardener · Nov 26, 2024 · a26df8d · a26df8d
1 parent 06080a2
commit a26df8d
Show file tree

Hide file tree

Showing 21 changed files with 107 additions and 331 deletions.
diff --git a/charts/druid/values.yaml b/charts/druid/values.yaml
@@ -11,8 +11,7 @@ resources:
     cpu: 50m
     memory: 128Mi
 
-featureGates:
-  UseEtcdWrapper: true
+featureGates: {}
 
 controllerManager:
   server:

diff --git a/docs/deployment/feature-gates.md b/docs/deployment/feature-gates.md
@@ -20,15 +20,16 @@ The following tables are a summary of the feature gates that you can set on etcd
 
 ## Feature Gates for Alpha or Beta Features
 
-| Feature          | Default | Stage   | Since  | Until |
-|------------------|---------|---------|--------|-------|
-| `UseEtcdWrapper` | `false` | `Alpha` | `0.19` | `0.21`|
-| `UseEtcdWrapper` | `true`  | `Beta`  | `0.22` |       |
+| Feature | Default | Stage | Since | Until |
+|---------|---------|-------|-------|-------|
 
 ## Feature Gates for Graduated or Deprecated Features
 
-| Feature | Default | Stage | Since | Until |
-|---------|---------|-------|-------|-------|
+| Feature          | Default | Stage   | Since  | Until  |
+|------------------|---------|---------|--------|--------|
+| `UseEtcdWrapper` | `false` | `Alpha` | `0.19` | `0.21` |
+| `UseEtcdWrapper` | `true`  | `Beta`  | `0.22` | `0.24` |
+| `UseEtcdWrapper` | `true`  | `GA`    | `0.25` |        |
 
 ## Using a Feature
 

diff --git a/internal/component/statefulset/builder.go b/internal/component/statefulset/builder.go
@@ -6,8 +6,6 @@ package statefulset
 
 import (
 	"fmt"
-	"strconv"
-	"strings"
 
 	druidv1alpha1 "github.com/gardener/etcd-druid/api/v1alpha1"
 	"github.com/gardener/etcd-druid/internal/common"
@@ -58,7 +56,6 @@ type stsBuilder struct {
 	client                 client.Client
 	etcd                   *druidv1alpha1.Etcd
 	replicas               int32
-	useEtcdWrapper         bool
 	provider               *string
 	etcdImage              string
 	etcdBackupRestoreImage string
@@ -79,11 +76,10 @@ func newStsBuilder(client client.Client,
 	logger logr.Logger,
 	etcd *druidv1alpha1.Etcd,
 	replicas int32,
-	useEtcdWrapper bool,
 	imageVector imagevector.ImageVector,
 	skipSetOrUpdateForbiddenFields bool,
 	sts *appsv1.StatefulSet) (*stsBuilder, error) {
-	etcdImage, etcdBackupRestoreImage, initContainerImage, err := utils.GetEtcdImages(etcd, imageVector, useEtcdWrapper)
+	etcdImage, etcdBackupRestoreImage, initContainerImage, err := utils.GetEtcdImages(etcd, imageVector)
 	if err != nil {
 		return nil, err
 	}
@@ -96,7 +92,6 @@ func newStsBuilder(client client.Client,
 		logger:                         logger,
 		etcd:                           etcd,
 		replicas:                       replicas,
-		useEtcdWrapper:                 useEtcdWrapper,
 		provider:                       provider,
 		etcdImage:                      etcdImage,
 		etcdBackupRestoreImage:         etcdBackupRestoreImage,
@@ -247,23 +242,7 @@ func (b *stsBuilder) getVolumeClaimTemplates() []corev1.PersistentVolumeClaim {
 }
 
 func (b *stsBuilder) getPodInitContainers() []corev1.Container {
-	initContainers := make([]corev1.Container, 0, 2)
-	if !b.useEtcdWrapper {
-		return initContainers
-	}
-	initContainers = append(initContainers, corev1.Container{
-		Name:            common.InitContainerNameChangePermissions,
-		Image:           b.initContainerImage,
-		ImagePullPolicy: corev1.PullIfNotPresent,
-		Command:         []string{"sh", "-c", "--"},
-		Args:            []string{fmt.Sprintf("chown -R %d:%d %s", nonRootUser, nonRootUser, common.VolumeMountPathEtcdData)},
-		VolumeMounts:    []corev1.VolumeMount{b.getEtcdDataVolumeMount()},
-		SecurityContext: &corev1.SecurityContext{
-			RunAsGroup:   ptr.To[int64](0),
-			RunAsNonRoot: ptr.To(false),
-			RunAsUser:    ptr.To[int64](0),
-		},
-	})
+	initContainers := make([]corev1.Container, 0, 1)
 	if b.etcd.IsBackupStoreEnabled() {
 		if b.provider != nil && *b.provider == druidstore.Local {
 			etcdBackupVolumeMount := b.getEtcdBackupVolumeMount()
@@ -344,16 +323,9 @@ func (b *stsBuilder) getEtcdBackupVolumeMount() *corev1.VolumeMount {
 	switch *b.provider {
 	case druidstore.Local:
 		if b.etcd.Spec.Backup.Store.Container != nil {
-			if b.useEtcdWrapper {
-				return &corev1.VolumeMount{
-					Name:      common.VolumeNameLocalBackup,
-					MountPath: fmt.Sprintf("/home/nonroot/%s", ptr.Deref(b.etcd.Spec.Backup.Store.Container, "")),
-				}
-			} else {
-				return &corev1.VolumeMount{
-					Name:      common.VolumeNameLocalBackup,
-					MountPath: ptr.Deref(b.etcd.Spec.Backup.Store.Container, ""),
-				}
+			return &corev1.VolumeMount{
+				Name:      common.VolumeNameLocalBackup,
+				MountPath: fmt.Sprintf("/home/nonroot/%s", ptr.Deref(b.etcd.Spec.Backup.Store.Container, "")),
 			}
 		}
 	case druidstore.GCS:
@@ -496,9 +468,7 @@ func (b *stsBuilder) getBackupRestoreContainerCommandArgs() []string {
 	commandArgs = append(commandArgs, fmt.Sprintf("--etcd-connection-timeout=%s", defaultEtcdConnectionTimeout))
 	commandArgs = append(commandArgs, "--enable-member-lease-renewal=true")
 	// Enable/Disable use Etcd Wrapper in BackupRestore container. Once `use-etcd-wrapper` feature-gate is GA then this value will always be true.
-	if b.useEtcdWrapper {
-		commandArgs = append(commandArgs, "--use-etcd-wrapper=true")
-	}
+	commandArgs = append(commandArgs, "--use-etcd-wrapper=true")
 
 	var quota = defaultQuota
 	if b.etcd.Spec.Etcd.Quota != nil {
@@ -601,13 +571,7 @@ func (b *stsBuilder) getEtcdContainerReadinessProbe() *corev1.Probe {
 
 func (b *stsBuilder) getEtcdContainerReadinessHandler() corev1.ProbeHandler {
 	multiNodeCluster := b.etcd.Spec.Replicas > 1
-	if multiNodeCluster && !b.useEtcdWrapper {
-		return corev1.ProbeHandler{
-			Exec: &corev1.ExecAction{
-				Command: b.getEtcdContainerReadinessProbeCommand(),
-			},
-		}
-	}
+
 	scheme := utils.IfConditionOr(b.etcd.Spec.Backup.TLS == nil, corev1.URISchemeHTTP, corev1.URISchemeHTTPS)
 	path := utils.IfConditionOr(multiNodeCluster, "/readyz", "/healthz")
 	port := utils.IfConditionOr(multiNodeCluster, common.DefaultPortEtcdWrapper, common.DefaultPortEtcdBackupRestore)
@@ -621,33 +585,7 @@ func (b *stsBuilder) getEtcdContainerReadinessHandler() corev1.ProbeHandler {
 	}
 }
 
-func (b *stsBuilder) getEtcdContainerReadinessProbeCommand() []string {
-	cmdBuilder := strings.Builder{}
-	cmdBuilder.WriteString("ETCDCTL_API=3 etcdctl")
-	if b.etcd.Spec.Etcd.ClientUrlTLS != nil {
-		dataKey := ptr.Deref(b.etcd.Spec.Etcd.ClientUrlTLS.TLSCASecretRef.DataKey, "ca.crt")
-		cmdBuilder.WriteString(fmt.Sprintf(" --cacert=%s/%s", common.VolumeMountPathEtcdCA, dataKey))
-		cmdBuilder.WriteString(fmt.Sprintf(" --cert=%s/tls.crt", common.VolumeMountPathEtcdClientTLS))
-		cmdBuilder.WriteString(fmt.Sprintf(" --key=%s/tls.key", common.VolumeMountPathEtcdClientTLS))
-		cmdBuilder.WriteString(fmt.Sprintf(" --endpoints=https://%s-local:%d", b.etcd.Name, b.clientPort))
-	} else {
-		cmdBuilder.WriteString(fmt.Sprintf(" --endpoints=http://%s-local:%d", b.etcd.Name, b.clientPort))
-	}
-	cmdBuilder.WriteString(" get foo")
-	cmdBuilder.WriteString(" --consistency=l")
-
-	return []string{
-		"/bin/sh",
-		"-ec",
-		cmdBuilder.String(),
-	}
-}
-
 func (b *stsBuilder) getEtcdContainerCommandArgs() []string {
-	if !b.useEtcdWrapper {
-		// safe to return an empty string array here since etcd-custom-image:v3.4.13-bootstrap-12 (as well as v3.4.26) now uses an entry point that calls bootstrap.sh
-		return []string{}
-	}
 	commandArgs := []string{"start-etcd"}
 	commandArgs = append(commandArgs, fmt.Sprintf("--backup-restore-host-port=%s-local:%d", b.etcd.Name, common.DefaultPortEtcdBackupRestore))
 	commandArgs = append(commandArgs, fmt.Sprintf("--etcd-server-name=%s-local", b.etcd.Name))
@@ -665,23 +603,10 @@ func (b *stsBuilder) getEtcdContainerCommandArgs() []string {
 }
 
 func (b *stsBuilder) getEtcdContainerEnvVars() []corev1.EnvVar {
-	if b.useEtcdWrapper {
-		return []corev1.EnvVar{}
-	}
-	backTLSEnabled := b.etcd.Spec.Backup.TLS != nil
-	scheme := utils.IfConditionOr(backTLSEnabled, "https", "http")
-	endpoint := fmt.Sprintf("%s://%s-local:%d", scheme, b.etcd.Name, b.backupPort)
-
-	return []corev1.EnvVar{
-		{Name: "ENABLE_TLS", Value: strconv.FormatBool(backTLSEnabled)},
-		{Name: "BACKUP_ENDPOINT", Value: endpoint},
-	}
+	return []corev1.EnvVar{}
 }
 
 func (b *stsBuilder) getPodSecurityContext() *corev1.PodSecurityContext {
-	if !b.useEtcdWrapper {
-		return nil
-	}
 	return &corev1.PodSecurityContext{
 		RunAsGroup:   ptr.To[int64](nonRootUser),
 		RunAsNonRoot: ptr.To(true),

diff --git a/internal/component/statefulset/statefulset.go b/internal/component/statefulset/statefulset.go
@@ -12,7 +12,6 @@ import (
 	"github.com/gardener/etcd-druid/internal/common"
 	"github.com/gardener/etcd-druid/internal/component"
 	druiderr "github.com/gardener/etcd-druid/internal/errors"
-	"github.com/gardener/etcd-druid/internal/features"
 	"github.com/gardener/etcd-druid/internal/utils"
 
 	"github.com/gardener/gardener/pkg/controllerutils"
@@ -23,7 +22,6 @@ import (
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
-	"k8s.io/component-base/featuregate"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
@@ -37,18 +35,16 @@ const (
 )
 
 type _resource struct {
-	client         client.Client
-	imageVector    imagevector.ImageVector
-	useEtcdWrapper bool
-	logger         logr.Logger
+	client      client.Client
+	imageVector imagevector.ImageVector
+	logger      logr.Logger
 }
 
 // New returns a new statefulset component operator.
-func New(client client.Client, imageVector imagevector.ImageVector, featureGates map[featuregate.Feature]bool) component.Operator {
+func New(client client.Client, imageVector imagevector.ImageVector) component.Operator {
 	return &_resource{
-		client:         client,
-		imageVector:    imageVector,
-		useEtcdWrapper: featureGates[features.UseEtcdWrapper],
+		client:      client,
+		imageVector: imageVector,
 	}
 }
 
@@ -225,7 +221,7 @@ func (r _resource) getExistingStatefulSet(ctx component.OperatorContext, etcdObj
 func (r _resource) createOrPatchWithReplicas(ctx component.OperatorContext, etcd *druidv1alpha1.Etcd, sts *appsv1.StatefulSet, replicas int32, skipSetOrUpdateForbiddenFields bool) error {
 	stsClone := sts.DeepCopy()
 	mutatingFn := func() error {
-		if builder, err := newStsBuilder(r.client, ctx.Logger, etcd, replicas, r.useEtcdWrapper, r.imageVector, skipSetOrUpdateForbiddenFields, stsClone); err != nil {
+		if builder, err := newStsBuilder(r.client, ctx.Logger, etcd, replicas, r.imageVector, skipSetOrUpdateForbiddenFields, stsClone); err != nil {
 			return err
 		} else {
 			return builder.Build(ctx)

diff --git a/internal/component/statefulset/statefulset_test.go b/internal/component/statefulset/statefulset_test.go
@@ -12,7 +12,6 @@ import (
 	"github.com/gardener/etcd-druid/internal/common"
 	"github.com/gardener/etcd-druid/internal/component"
 	druiderr "github.com/gardener/etcd-druid/internal/errors"
-	"github.com/gardener/etcd-druid/internal/features"
 	druidstore "github.com/gardener/etcd-druid/internal/store"
 	"github.com/gardener/etcd-druid/internal/utils"
 	testutils "github.com/gardener/etcd-druid/test/utils"
@@ -23,7 +22,6 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/component-base/featuregate"
 	"k8s.io/utils/ptr"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
@@ -72,7 +70,7 @@ func TestGetExistingResourceNames(t *testing.T) {
 				existingObjects = append(existingObjects, emptyStatefulSet(etcd.ObjectMeta))
 			}
 			cl := testutils.CreateTestFakeClientForObjects(tc.getErr, nil, nil, nil, existingObjects, getObjectKey(etcd.ObjectMeta))
-			operator := New(cl, nil, nil)
+			operator := New(cl, nil)
 			opCtx := component.NewOperatorContext(context.Background(), logr.Discard(), uuid.NewString())
 			actualStsNames, err := operator.GetExistingResourceNames(opCtx, etcd.ObjectMeta)
 			if tc.expectedErr != nil {
@@ -115,19 +113,17 @@ func TestSyncWhenNoSTSExists(t *testing.T) {
 
 	g := NewWithT(t)
 	t.Parallel()
-	iv := testutils.CreateImageVector(false, false, true, true)
+	iv := testutils.CreateImageVector(true, true)
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
 			t.Parallel()
 			// *************** Build test environment ***************
 			etcd := testutils.EtcdBuilderWithDefaults(testutils.TestEtcdName, testutils.TestNamespace).WithReplicas(tc.replicas).Build()
 			cl := testutils.CreateTestFakeClientForObjects(nil, tc.createErr, nil, nil, []client.Object{buildBackupSecret()}, getObjectKey(etcd.ObjectMeta))
-			etcdImage, etcdBRImage, initContainerImage, err := utils.GetEtcdImages(etcd, iv, true)
+			etcdImage, etcdBRImage, initContainerImage, err := utils.GetEtcdImages(etcd, iv)
 			g.Expect(err).ToNot(HaveOccurred())
-			stsMatcher := NewStatefulSetMatcher(g, cl, etcd, tc.replicas, true, initContainerImage, etcdImage, etcdBRImage, ptr.To(druidstore.Local))
-			operator := New(cl, iv, map[featuregate.Feature]bool{
-				features.UseEtcdWrapper: true,
-			})
+			stsMatcher := NewStatefulSetMatcher(g, cl, etcd, tc.replicas, initContainerImage, etcdImage, etcdBRImage, ptr.To(druidstore.Local))
+			operator := New(cl, iv)
 			// *************** Test and assert ***************
 			opCtx := component.NewOperatorContext(context.Background(), logr.Discard(), uuid.NewString())
 			opCtx.Data[common.CheckSumKeyConfigMap] = testutils.TestConfigMapCheckSum