Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Promote UseEtcdWrapper feature gate to GA #936

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions charts/druid/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,7 @@ resources:
cpu: 50m
memory: 128Mi

featureGates:
UseEtcdWrapper: true
featureGates: {}

controllerManager:
server:
Expand Down
13 changes: 7 additions & 6 deletions docs/deployment/feature-gates.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
91 changes: 8 additions & 83 deletions internal/component/statefulset/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -58,7 +56,6 @@ type stsBuilder struct {
client client.Client
etcd *druidv1alpha1.Etcd
replicas int32
useEtcdWrapper bool
provider *string
etcdImage string
etcdBackupRestoreImage string
Expand All @@ -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
}
Expand All @@ -96,7 +92,6 @@ func newStsBuilder(client client.Client,
logger: logger,
etcd: etcd,
replicas: replicas,
useEtcdWrapper: useEtcdWrapper,
provider: provider,
etcdImage: etcdImage,
etcdBackupRestoreImage: etcdBackupRestoreImage,
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
Expand All @@ -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))
Expand All @@ -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),
Expand Down
18 changes: 7 additions & 11 deletions internal/component/statefulset/statefulset.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"
)

Expand All @@ -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,
}
}

Expand Down Expand Up @@ -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)
Expand Down
14 changes: 5 additions & 9 deletions internal/component/statefulset/statefulset_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down
Loading