Skip to content

Commit

Permalink
Merge pull request #1352 from tjungblu/ETCD-677
Browse files Browse the repository at this point in the history
ETCD-677: add unsupported config override for etcd container removal
  • Loading branch information
openshift-merge-bot[bot] authored Dec 3, 2024
2 parents 47b36cf + 87714bf commit 658cb7e
Show file tree
Hide file tree
Showing 6 changed files with 174 additions and 115 deletions.
2 changes: 2 additions & 0 deletions bindata/etcd/pod.gotpl.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ spec:
{{ end -}}
- name: "ETCD_STATIC_POD_VERSION"
value: "REVISION"
{{ if .EnableEtcdContainer }}
- name: etcd
image: {{.Image}}
imagePullPolicy: IfNotPresent
Expand Down Expand Up @@ -244,6 +245,7 @@ spec:
name: cert-dir
- mountPath: /var/lib/etcd/
name: data-dir
{{ end }}
- name: etcd-metrics
image: {{.Image}}
imagePullPolicy: IfNotPresent
Expand Down
15 changes: 11 additions & 4 deletions pkg/operator/ceohelpers/unsupported_override.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,17 @@ import (
"k8s.io/klog/v2"
)

// isUnsupportedUnsafeEtcd returns true if
// useUnsupportedUnsafeNonHANonProductionUnstableEtcd key is set
// to any parsable value
// isUnsupportedUnsafeEtcd returns true if useUnsupportedUnsafeNonHANonProductionUnstableEtcd key is set to any parsable value
func isUnsupportedUnsafeEtcd(spec *operatorv1.StaticPodOperatorSpec) (bool, error) {
return tryGetUnsupportedValue(spec, "useUnsupportedUnsafeNonHANonProductionUnstableEtcd")
}

// IsUnsupportedUnsafeEtcdContainerRemoval returns true if useUnsupportedUnsafeEtcdContainerRemoval is set to any parsable value.
func IsUnsupportedUnsafeEtcdContainerRemoval(spec *operatorv1.StaticPodOperatorSpec) (bool, error) {
return tryGetUnsupportedValue(spec, "useUnsupportedUnsafeEtcdContainerRemoval")
}

func tryGetUnsupportedValue(spec *operatorv1.StaticPodOperatorSpec, key string) (bool, error) {
unsupportedConfig := map[string]interface{}{}
if spec.UnsupportedConfigOverrides.Raw == nil {
return false, nil
Expand All @@ -40,7 +47,7 @@ func isUnsupportedUnsafeEtcd(spec *operatorv1.StaticPodOperatorSpec) (bool, erro
// unstable
// 4. the combination of all these things makes the situation
// unsupportable.
value, found, err := unstructured.NestedFieldNoCopy(unsupportedConfig, "useUnsupportedUnsafeNonHANonProductionUnstableEtcd")
value, found, err := unstructured.NestedFieldNoCopy(unsupportedConfig, key)
if err != nil {
return false, err
}
Expand Down
110 changes: 36 additions & 74 deletions pkg/operator/ceohelpers/unsupported_override_test.go
Original file line number Diff line number Diff line change
@@ -1,118 +1,80 @@
package ceohelpers

import (
"testing"

"fmt"
"github.com/stretchr/testify/require"
"k8s.io/apimachinery/pkg/runtime"
"testing"

operatorv1 "github.com/openshift/api/operator/v1"
)

func TestIsUnsupportedUnsafeEtcd(t *testing.T) {
type args struct {
spec *operatorv1.StaticPodOperatorSpec
}
func TestIsUnsupportedOptions(t *testing.T) {
options := []string{"useUnsupportedUnsafeNonHANonProductionUnstableEtcd", "useUnsupportedUnsafeEtcdContainerRemoval"}
tests := []struct {
name string
args args
args string
want bool
wantErr bool
}{
{
name: "test value with a bool",
args: args{spec: &operatorv1.StaticPodOperatorSpec{
OperatorSpec: operatorv1.OperatorSpec{
UnsupportedConfigOverrides: runtime.RawExtension{
Raw: []byte("useUnsupportedUnsafeNonHANonProductionUnstableEtcd: true"),
},
},
}},
name: "test value with a bool",
args: "%s: true",
want: true,
wantErr: false,
},
{
name: "test value=true with a string",
args: args{spec: &operatorv1.StaticPodOperatorSpec{
OperatorSpec: operatorv1.OperatorSpec{
UnsupportedConfigOverrides: runtime.RawExtension{
Raw: []byte(`useUnsupportedUnsafeNonHANonProductionUnstableEtcd: "true"`),
},
},
}},
name: "test value=true with a string",
args: `%s: "true"`,
want: true,
wantErr: false,
},
{
name: "test value=false with a string",
args: args{spec: &operatorv1.StaticPodOperatorSpec{
OperatorSpec: operatorv1.OperatorSpec{
UnsupportedConfigOverrides: runtime.RawExtension{
Raw: []byte(`useUnsupportedUnsafeNonHANonProductionUnstableEtcd: "false"`),
},
},
}},
name: "test value=false with a string",
args: `%s: "false"`,
want: false,
wantErr: false,
},
{
name: "test value=true with a json bool",
args: args{spec: &operatorv1.StaticPodOperatorSpec{
OperatorSpec: operatorv1.OperatorSpec{
UnsupportedConfigOverrides: runtime.RawExtension{
Raw: []byte(`{"useUnsupportedUnsafeNonHANonProductionUnstableEtcd": true}`),
},
},
}},
name: "test value=true with a json bool",
args: `{"%s": true}`,
want: true,
wantErr: false,
},
{
name: "test value=true with a json string",
args: args{spec: &operatorv1.StaticPodOperatorSpec{
OperatorSpec: operatorv1.OperatorSpec{
UnsupportedConfigOverrides: runtime.RawExtension{
Raw: []byte(`{"useUnsupportedUnsafeNonHANonProductionUnstableEtcd": "true"}`),
},
},
}},
name: "test value=true with a json string",
args: `{"%s": "true"}`,
want: true,
wantErr: false,
},
{
name: "test value=false with a json string",
args: args{spec: &operatorv1.StaticPodOperatorSpec{
OperatorSpec: operatorv1.OperatorSpec{
UnsupportedConfigOverrides: runtime.RawExtension{
Raw: []byte(`{"useUnsupportedUnsafeNonHANonProductionUnstableEtcd": "false"}`),
},
},
}},
name: "test value=false with a json string",
args: `{"%s": "false"}`,
want: false,
wantErr: false,
},
{
name: "test value=false with a json string",
args: args{spec: &operatorv1.StaticPodOperatorSpec{
OperatorSpec: operatorv1.OperatorSpec{
UnsupportedConfigOverrides: runtime.RawExtension{
Raw: []byte(`{"useUnsupportedUnsafeNonHANonProductionUnstableEtcd": "randomValue"}`),
},
},
}},
name: "test value=false with a json string",
args: `{"%s": "randomValue"}`,
want: false,
wantErr: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got, err := isUnsupportedUnsafeEtcd(tt.args.spec)
if (err != nil) != tt.wantErr {
t.Errorf("IsUnsupportedUnsafeEtcd() error = %v, wantErr %v", err, tt.wantErr)
return
}
if got != tt.want {
t.Errorf("IsUnsupportedUnsafeEtcd() got = %v, want %v", got, tt.want)
}
})

for _, option := range options {
for _, tt := range tests {
t.Run(fmt.Sprintf("%s/%s", option, tt.name), func(t *testing.T) {
spec := &operatorv1.StaticPodOperatorSpec{
OperatorSpec: operatorv1.OperatorSpec{
UnsupportedConfigOverrides: runtime.RawExtension{
Raw: []byte(fmt.Sprintf(tt.args, option)),
},
},
}
got, err := tryGetUnsupportedValue(spec, option)
require.Equal(t, tt.wantErr, err != nil)
require.Equal(t, tt.want, got)
})
}
}
}
2 changes: 2 additions & 0 deletions pkg/operator/etcd_assets/bindata.go

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

47 changes: 29 additions & 18 deletions pkg/operator/targetconfigcontroller/targetconfigcontroller.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"errors"
"fmt"
"github.com/openshift/cluster-etcd-operator/pkg/operator/ceohelpers"
"reflect"
"strings"
"text/template"
Expand Down Expand Up @@ -37,14 +38,15 @@ type NameValue struct {
}

type PodSubstitutionTemplate struct {
Image string
OperatorImage string
ListenAddress string
LocalhostAddress string
LogLevel string
EnvVars []NameValue
BackupArgs []string
CipherSuites string
Image string
OperatorImage string
ListenAddress string
LocalhostAddress string
LogLevel string
EnvVars []NameValue
BackupArgs []string
CipherSuites string
EnableEtcdContainer bool
}

type TargetConfigController struct {
Expand Down Expand Up @@ -159,7 +161,11 @@ func (c *TargetConfigController) createTargetConfig(
if err != nil {
return err
}
podSub := c.getPodSubstitution(operatorSpec, c.targetImagePullSpec, c.operatorImagePullSpec, envVars)
podSub, err := c.getPodSubstitution(operatorSpec, c.targetImagePullSpec, c.operatorImagePullSpec, envVars)
if err != nil {
return err
}

_, _, err = c.manageStandardPod(ctx, podSub, c.kubeClient.CoreV1(), recorder, operatorSpec)
if err != nil {
errs = errors.Join(errs, fmt.Errorf("%q: %w", "configmap/etcd-pod", err))
Expand Down Expand Up @@ -202,7 +208,11 @@ func (c *TargetConfigController) getSubstitutionReplacer(operatorSpec *operatorv
}

func (c *TargetConfigController) getPodSubstitution(operatorSpec *operatorv1.StaticPodOperatorSpec,
imagePullSpec, operatorImagePullSpec string, envVarMap map[string]string) *PodSubstitutionTemplate {
imagePullSpec, operatorImagePullSpec string, envVarMap map[string]string) (*PodSubstitutionTemplate, error) {
shouldRemoveEtcdContainer, err := ceohelpers.IsUnsupportedUnsafeEtcdContainerRemoval(operatorSpec)
if err != nil {
return nil, err
}

var nameValues []NameValue
for _, k := range sets.StringKeySet(envVarMap).List() {
Expand All @@ -211,14 +221,15 @@ func (c *TargetConfigController) getPodSubstitution(operatorSpec *operatorv1.Sta
}

return &PodSubstitutionTemplate{
Image: imagePullSpec,
OperatorImage: operatorImagePullSpec,
ListenAddress: "0.0.0.0", // TODO this needs updating to detect ipv6-ness
LocalhostAddress: "127.0.0.1", // TODO this needs updating to detect ipv6-ness
LogLevel: loglevelToZap(operatorSpec.LogLevel),
EnvVars: nameValues,
CipherSuites: envVarMap["ETCD_CIPHER_SUITES"],
}
Image: imagePullSpec,
OperatorImage: operatorImagePullSpec,
ListenAddress: "0.0.0.0", // TODO this needs updating to detect ipv6-ness
LocalhostAddress: "127.0.0.1", // TODO this needs updating to detect ipv6-ness
LogLevel: loglevelToZap(operatorSpec.LogLevel),
EnvVars: nameValues,
CipherSuites: envVarMap["ETCD_CIPHER_SUITES"],
EnableEtcdContainer: !shouldRemoveEtcdContainer,
}, nil
}

func (c *TargetConfigController) manageRecoveryPods(
Expand Down
Loading

0 comments on commit 658cb7e

Please sign in to comment.