Refactor components to use default labels and owner references from `…

…etcd` (#559)

* Refactor: Update `configmap` component to use default labels and owner references from `etcd`.

This commit refactors the `configmap` component to use the default labels and owner references provided by etcd. Specifically, we removed the functions `getOwnerReferences` and `getObjectMeta`, and instead used the `GetEtcdAsOwnerReference` and `GetDefaultLabels` functions.

In addition, we improved the formulation of the configmap by adding a new function called `getEtcdConfigYaml`.

* Refactor: Update `lease` component to use default labels and owner references from `etcd`.

This commit refactors the `lease` component to use the default labels and owner references provided by etcd. Specifically, we removed the functions `getOwnerReferences`, and instead used the `GetEtcdAsOwnerReference` and `GetDefaultLabels` functions.

* Refactor: Update `poddisruptionbudget` component to use default labels and owner references from `etcd`.

This commit refactors the `poddisruptionbudget` component to use the default labels and owner references provided by etcd. Specifically, we removed the functions `getOwnerReferences`, and instead used the `GetEtcdAsOwnerReference` and `GetDefaultLabels` functions.

* Refactor: Update `service` component to use default labels and owner references from `etcd`.

This commit refactors the `service` component to use the default labels and owner references provided by etcd. Specifically, removed the functions `getOwnerReferences`, `getSelectors`, `getLabels` and instead used the `GetEtcdAsOwnerReference` and `GetDefaultLabels` functions.

* Refactor: Update `statefulset` component to use default labels and owner references from `etcd`.

This commit refactors the `statefulset` component to use the default labels and owner references provided by etcd. Specifically, removed the functions `getCommonLabels` and instead used the `GetAsOwnerReference` and `GetDefaultLabels` functions.

Fix intergration test

rebase with master

* Updated the E2E test suite to use the appropriate component names.

* Addressed review comment from Madhav about OwnerReferences.

This commit addresses the review comment #539 (comment)

* This commit addresses the review comment from Shreyas about improving the service account test in response to #538 (review)

* Refactor API version to use 'druidv1alpha1.GroupVersion.String()' instead of the static string 'druid.gardener.cloud/v1alpha1

* Addressed review comment

- Updated function `GetAsOwnerReference to return metav1.OwnerReference instead of *metav1.OwnerReference

* Renamed function name checkConfigmap to checkConfigMap

* Renamed ServerPort to PeerPort

* Removed arguments of emptyPodDisruptionBudget and reused name, namespace from component's value

* Constructing a etcdconfig YAML using strut instead of string string concatenation

* Used default labels for lease component

* Update log message with OwnerReference <name>:<uid>

* make revendor

* fix failing test

* Removed checkPDBMetadata function

* minor refactoring to configmap component

* Removed JSON tags

* Rebase with master

* Fix integration test

* Fix failing test

* Apply suggestions from code review

Co-authored-by: Aaron Francis Fernandes <79958509+aaronfern@users.noreply.github.com>

* Addressed review feedback

* Apply suggestions from code review

Co-authored-by: Shreyas Rao <42259948+shreyas-s-rao@users.noreply.github.com>

* Addressed review comment
- Renamed field `PodAdditionalLabels` to `AdditionalPodLabels`.

* Update pkg/component/etcd/configmap/values.go

Co-authored-by: Shreyas Rao <42259948+shreyas-s-rao@users.noreply.github.com>

* Fixed typo

---------

Co-authored-by: Madhav Bhargava <madhav.bhargava@sap.com>
Co-authored-by: Aaron Francis Fernandes <79958509+aaronfern@users.noreply.github.com>
Co-authored-by: Shreyas Rao <42259948+shreyas-s-rao@users.noreply.github.com>

api/v1alpha1/types_etcd_test.go

@@ -129,7 +129,6 @@ var _ = Describe("Etcd", func() {
 
 	Context("GetAsOwnerReference", func() {
 		It("should return an OwnerReference object that represents the current Etcd instance", func() {
-
 			expected := metav1.OwnerReference{
 				APIVersion:         GroupVersion.String(),
 				Kind:               "Etcd",

controllers/compaction/reconciler.go

@@ -268,7 +268,7 @@ func (r *Reconciler) createCompactionJob(ctx context.Context, logger logr.Logger
 			Labels:    getLabels(etcd),
 			OwnerReferences: []metav1.OwnerReference{
 				{
-					APIVersion:         "druid.gardener.cloud/v1alpha1",
+					APIVersion:         druidv1alpha1.GroupVersion.String(),
 					BlockOwnerDeletion: pointer.Bool(true),
 					Controller:         pointer.Bool(true),
 					Kind:               "Etcd",

controllers/etcd/reconciler.go

@@ -369,7 +369,7 @@ func (r *Reconciler) reconcileEtcd(ctx context.Context, logger logr.Logger, etcd
 	peerUrlTLSChangedToEnabled := isPeerTLSChangedToEnabled(peerTLSEnabled, configMapValues)
 	statefulSetValues := componentsts.GenerateValues(etcd,
 		&serviceValues.ClientPort,
-		&serviceValues.ServerPort,
+		&serviceValues.PeerPort,
 		&serviceValues.BackupPort,
 		*etcdImage,
 		*etcdBackupImage,

controllers/etcdcopybackupstask/reconciler_test.go

@@ -720,7 +720,7 @@ func matchJob(task *druidv1alpha1.EtcdCopyBackupsTask, imageVector imagevector.I
 			}),
 			"OwnerReferences": MatchAllElements(testutils.OwnerRefIterator, Elements{
 				task.Name: MatchAllFields(Fields{
-					"APIVersion":         Equal("druid.gardener.cloud/v1alpha1"),
+					"APIVersion":         Equal(druidv1alpha1.GroupVersion.String()),
 					"Kind":               Equal("EtcdCopyBackupsTask"),
 					"Name":               Equal(task.Name),
 					"UID":                Equal(task.UID),

go.mod

@@ -14,6 +14,7 @@ require (
 	github.com/prometheus/client_golang v1.14.0
 	go.uber.org/zap v1.24.0
 	golang.org/x/exp v0.0.0-20230213192124-5e25df0256eb
+	gopkg.in/yaml.v2 v2.4.0
 	k8s.io/api v0.26.1
 	k8s.io/apimachinery v0.26.1
 	k8s.io/client-go v11.0.1-0.20190409021438-1a26190bd76a+incompatible
@@ -118,7 +119,6 @@ require (
 	google.golang.org/grpc v1.49.0 // indirect
 	google.golang.org/protobuf v1.28.1 // indirect
 	gopkg.in/inf.v0 v0.9.1 // indirect
-	gopkg.in/yaml.v2 v2.4.0 // indirect
 	gopkg.in/yaml.v3 v3.0.1 // indirect
 	istio.io/api v0.0.0-20221013011440-bc935762d2b9 // indirect
 	istio.io/client-go v1.15.3 // indirect

pkg/component/etcd/configmap/configmap.go

@@ -18,17 +18,17 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
-	"strconv"
 
-	druidv1alpha1 "github.com/gardener/etcd-druid/api/v1alpha1"
-	corev1 "k8s.io/api/core/v1"
-
-	"github.com/gardener/gardener/pkg/controllerutils"
 	gardenercomponent "github.com/gardener/gardener/pkg/operation/botanist/component"
-	"github.com/gardener/gardener/pkg/utils"
+	"gopkg.in/yaml.v2"
+	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/utils/pointer"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+
+	druidv1alpha1 "github.com/gardener/etcd-druid/api/v1alpha1"
+	"github.com/gardener/gardener/pkg/controllerutils"
+	"github.com/gardener/gardener/pkg/utils"
 )
 
 type component struct {
@@ -48,146 +48,84 @@ func New(c client.Client, namespace string, values *Values) gardenercomponent.De
 }
 
 func (c *component) Deploy(ctx context.Context) error {
-	cm := c.emptyConfigmap(c.values.ConfigMapName)
-
+	cm := c.emptyConfigmap()
 	if err := c.syncConfigmap(ctx, cm); err != nil {
 		return err
 	}
-
 	return nil
 }
 
-func (c *component) syncConfigmap(ctx context.Context, cm *corev1.ConfigMap) error {
-	etcdConfigYaml := `# Human-readable name for this member.
-    name: ` + fmt.Sprintf("etcd-%s", c.values.EtcdUID[:6]) + `
-    # Path to the data directory.
-    data-dir: /var/etcd/data/new.etcd`
-	metricsLevel := druidv1alpha1.Basic
-	if c.values.Metrics != nil {
-		metricsLevel = *c.values.Metrics
+func (c *component) Destroy(ctx context.Context) error {
+	configMap := c.emptyConfigmap()
+	if err := c.deleteConfigmap(ctx, configMap); err != nil {
+		return err
 	}
+	return nil
+}
 
-	etcdConfigYaml = etcdConfigYaml + `
-    # metrics configuration
-    metrics: ` + string(metricsLevel) + `
-    # Number of committed transactions to trigger a snapshot to disk.
-    snapshot-count: 75000
+type etcdTLSTarget string
 
-    # Accept etcd V2 client requests
-    enable-v2: false`
-	var quota int64 = 8 * 1024 * 1024 * 1024 // 8Gi
+const (
+	clientTLS                  etcdTLSTarget = "client"
+	peerTLS                    etcdTLSTarget = "peer"
+	defaultQuota                             = int64(8 * 1024 * 1024 * 1024) // 8Gi
+	defaultSnapshotCount                     = 75000                         // for more information refer to https://etcd.io/docs/v3.4/op-guide/maintenance/#raft-log-retention
+	defaultInitialClusterState               = "new"
+	defaultInitialClusterToken               = "etcd-cluster"
+)
+
+func (c *component) syncConfigmap(ctx context.Context, cm *corev1.ConfigMap) error {
+	peerScheme, peerSecurity := getSchemeAndSecurityConfig(c.values.PeerUrlTLS, peerTLS)
+	clientScheme, clientSecurity := getSchemeAndSecurityConfig(c.values.ClientUrlTLS, clientTLS)
+
+	quota := defaultQuota
 	if c.values.Quota != nil {
 		quota = c.values.Quota.Value()
 	}
 
-	etcdConfigYaml = etcdConfigYaml + `
-    # Raise alarms when backend size exceeds the given quota. 0 means use the
-    # default quota.
-    quota-backend-bytes: ` + fmt.Sprint(quota)
-
-	enableTLS := (c.values.ClientUrlTLS != nil)
-
-	protocol := "http"
-	if enableTLS {
-		protocol = "https"
-		tlsCASecretKey := "ca.crt"
-		if dataKey := c.values.ClientUrlTLS.TLSCASecretRef.DataKey; dataKey != nil {
-			tlsCASecretKey = *dataKey
-		}
-		etcdConfigYaml = etcdConfigYaml + `
-    client-transport-security:
-        # Path to the client server TLS cert file.
-        cert-file: /var/etcd/ssl/client/server/tls.crt
-
-        # Path to the client server TLS key file.
-        key-file: /var/etcd/ssl/client/server/tls.key
-
-        # Enable client cert authentication.
-        client-cert-auth: true
-
-        # Path to the client server TLS trusted CA cert file.
-        trusted-ca-file: /var/etcd/ssl/client/ca/` + tlsCASecretKey + `
-
-        # Client TLS using generated certificates
-        auto-tls: false`
+	autoCompactionMode := string(druidv1alpha1.Periodic)
+	if c.values.AutoCompactionMode != nil {
+		autoCompactionMode = string(*c.values.AutoCompactionMode)
 	}
 
-	clientPort := strconv.Itoa(int(pointer.Int32Deref(c.values.ClientPort, defaultClientPort)))
-
-	serverPort := strconv.Itoa(int(pointer.Int32Deref(c.values.ServerPort, defaultServerPort)))
-
-	etcdConfigYaml = etcdConfigYaml + `
-    # List of comma separated URLs to listen on for client traffic.
-    listen-client-urls: ` + protocol + `://0.0.0.0:` + clientPort + `
-
-    # List of this member's client URLs to advertise to the public.
-    # The URLs needed to be a comma-separated list.
-    advertise-client-urls: ` + protocol + `@` + c.values.PeerServiceName + `@` + c.values.EtcdNameSpace + `@` + clientPort
-
-	enableTLS = (c.values.PeerUrlTLS != nil)
-	protocol = "http"
-	if enableTLS {
-		protocol = "https"
-		tlsCASecretKey := "ca.crt"
-		if dataKey := c.values.PeerUrlTLS.TLSCASecretRef.DataKey; dataKey != nil {
-			tlsCASecretKey = *dataKey
-		}
-		etcdConfigYaml = etcdConfigYaml + `
-    peer-transport-security:
-        # Path to the peer server TLS cert file.
-        cert-file: /var/etcd/ssl/peer/server/tls.crt
-
-        # Path to the peer server TLS key file.
-        key-file: /var/etcd/ssl/peer/server/tls.key
-
-        # Enable peer cert authentication.
-        client-cert-auth: true
-
-        # Path to the peer server TLS trusted CA cert file.
-        trusted-ca-file: /var/etcd/ssl/peer/ca/` + tlsCASecretKey + `
-
-        # Peer TLS using generated certificates
-        auto-tls: false`
+	etcdConfig := &etcdConfig{
+		Name:                    fmt.Sprintf("etcd-%s", c.values.EtcdUID[:6]),
+		DataDir:                 "/var/etcd/data/new.etcd",
+		Metrics:                 string(druidv1alpha1.Basic),
+		SnapshotCount:           defaultSnapshotCount,
+		EnableV2:                false,
+		QuotaBackendBytes:       quota,
+		InitialClusterToken:     defaultInitialClusterToken,
+		InitialClusterState:     defaultInitialClusterState,
+		InitialCluster:          c.values.InitialCluster,
+		AutoCompactionMode:      autoCompactionMode,
+		AutoCompactionRetention: pointer.StringDeref(c.values.AutoCompactionRetention, "30m"),
+
+		ListenPeerUrls:      fmt.Sprintf("%s://0.0.0.0:%d", peerScheme, pointer.Int32Deref(c.values.ServerPort, defaultServerPort)),
+		ListenClientUrls:    fmt.Sprintf("%s://0.0.0.0:%d", clientScheme, pointer.Int32Deref(c.values.ClientPort, defaultClientPort)),
+		AdvertisePeerUrls:   fmt.Sprintf("%s@%s@%s@%d", peerScheme, c.values.PeerServiceName, c.namespace, pointer.Int32Deref(c.values.ServerPort, defaultServerPort)),
+		AdvertiseClientUrls: fmt.Sprintf("%s@%s@%s@%d", clientScheme, c.values.PeerServiceName, c.namespace, pointer.Int32Deref(c.values.ClientPort, defaultClientPort)),
 	}
 
-	etcdConfigYaml = etcdConfigYaml + `
-    # List of comma separated URLs to listen on for peer traffic.
-    listen-peer-urls: ` + protocol + `://0.0.0.0:` + serverPort + `
-
-    # List of this member's peer URLs to advertise to the public.
-    # The URLs needed to be a comma-separated list.
-    initial-advertise-peer-urls: ` + protocol + `@` + c.values.PeerServiceName + `@` + c.values.EtcdNameSpace + `@` + serverPort + `
-
-    # Initial cluster token for the etcd cluster during bootstrap.
-    initial-cluster-token: etcd-cluster
-
-    # Initial cluster state ('new' or 'existing').
-    initial-cluster-state: new
-
-    # Initial cluster
-    initial-cluster: ` + c.values.InitialCluster + `
-
-    # auto-compaction-mode ("periodic" or "revision").`
+	if clientSecurity != nil {
+		etcdConfig.ClientSecurity = *clientSecurity
+	}
 
-	autoCompactionMode := "periodic"
-	if c.values.AutoCompactionMode != nil {
-		autoCompactionMode = string(*c.values.AutoCompactionMode)
+	if peerSecurity != nil {
+		etcdConfig.PeerSecurity = *peerSecurity
 	}
-	etcdConfigYaml = etcdConfigYaml + `
-    auto-compaction-mode: ` + autoCompactionMode
 
-	autoCompactionRetention := "30m"
-	if c.values.AutoCompactionRetention != nil {
-		autoCompactionRetention = string(*c.values.AutoCompactionRetention)
+	etcdConfigYaml, err := yaml.Marshal(etcdConfig)
+	if err != nil {
+		return err
 	}
-	etcdConfigYaml = etcdConfigYaml + `
-    # auto-compaction-retention defines Auto compaction retention length for etcd.
-    auto-compaction-retention: ` + autoCompactionRetention
-
-	_, err := controllerutils.GetAndCreateOrMergePatch(ctx, c.client, cm, func() error {
-		cm.ObjectMeta = getObjectMeta(c.values)
-		cm.OwnerReferences = getOwnerReferences(c.values)
-		cm.Data = map[string]string{"etcd.conf.yaml": etcdConfigYaml}
+
+	_, err = controllerutils.GetAndCreateOrMergePatch(ctx, c.client, cm, func() error {
+		cm.Name = c.values.Name
+		cm.Namespace = c.namespace
+		cm.Labels = c.values.Labels
+		cm.OwnerReferences = []metav1.OwnerReference{c.values.OwnerReference}
+		cm.Data = map[string]string{"etcd.conf.yaml": string(etcdConfigYaml)}
 		return nil
 	})
 	if err != nil {
@@ -200,54 +138,63 @@ func (c *component) syncConfigmap(ctx context.Context, cm *corev1.ConfigMap) err
 		return err
 	}
 	c.values.ConfigMapChecksum = utils.ComputeSHA256Hex(jsonString)
-
-	return nil
-
-}
-
-func (c *component) Destroy(ctx context.Context) error {
-	configMap := c.emptyConfigmap(c.values.ConfigMapName)
-
-	if err := c.deleteConfigmap(ctx, configMap); err != nil {
-		return err
-	}
 	return nil
 }
 
 func (c *component) deleteConfigmap(ctx context.Context, cm *corev1.ConfigMap) error {
 	return client.IgnoreNotFound(c.client.Delete(ctx, cm))
 }
 
-func (c *component) emptyConfigmap(name string) *corev1.ConfigMap {
+func (c *component) emptyConfigmap() *corev1.ConfigMap {
 	return &corev1.ConfigMap{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:      name,
+			Name:      c.values.Name,
 			Namespace: c.namespace,
 		},
 	}
 }
 
-func getObjectMeta(val *Values) metav1.ObjectMeta {
-	labels := map[string]string{"name": "etcd", "instance": val.EtcdName}
-	for key, value := range val.Labels {
-		labels[key] = value
-	}
-	return metav1.ObjectMeta{
-		Name:      val.ConfigMapName,
-		Namespace: val.EtcdNameSpace,
-		Labels:    labels,
-	}
+type etcdConfig struct {
+	Name                    string         `yaml:"name"`
+	DataDir                 string         `yaml:"data-dir"`
+	Metrics                 string         `yaml:"metrics"`
+	SnapshotCount           int            `yaml:"snapshot-count"`
+	EnableV2                bool           `yaml:"enable-v2"`
+	QuotaBackendBytes       int64          `yaml:"quota-backend-bytes"`
+	InitialClusterToken     string         `yaml:"initial-cluster-token"`
+	InitialClusterState     string         `yaml:"initial-cluster-state"`
+	InitialCluster          string         `yaml:"initial-cluster"`
+	AutoCompactionMode      string         `yaml:"auto-compaction-mode"`
+	AutoCompactionRetention string         `yaml:"auto-compaction-retention"`
+	ListenPeerUrls          string         `yaml:"listen-peer-urls"`
+	ListenClientUrls        string         `yaml:"listen-client-urls"`
+	AdvertisePeerUrls       string         `yaml:"initial-advertise-peer-urls"`
+	AdvertiseClientUrls     string         `yaml:"advertise-client-urls"`
+	ClientSecurity          securityConfig `yaml:"client-transport-security,omitempty"`
+	PeerSecurity            securityConfig `yaml:"peer-transport-security,omitempty"`
 }
 
-func getOwnerReferences(val *Values) []metav1.OwnerReference {
-	return []metav1.OwnerReference{
-		{
-			APIVersion:         druidv1alpha1.GroupVersion.String(),
-			Kind:               "Etcd",
-			Name:               val.EtcdName,
-			UID:                val.EtcdUID,
-			Controller:         pointer.Bool(true),
-			BlockOwnerDeletion: pointer.Bool(true),
-		},
+type securityConfig struct {
+	CertFile       string `yaml:"cert-file,omitempty"`
+	KeyFile        string `yaml:"key-file,omitempty"`
+	ClientCertAuth bool   `yaml:"client-cert-auth,omitempty"`
+	TrustedCAFile  string `yaml:"trusted-ca-file,omitempty"`
+	AutoTLS        bool   `yaml:"auto-tls"`
+}
+
+func getSchemeAndSecurityConfig(tlsConfig *druidv1alpha1.TLSConfig, tlsTarget etcdTLSTarget) (string, *securityConfig) {
+	if tlsConfig != nil {
+		tlsCASecretKey := "ca.crt"
+		if dataKey := tlsConfig.TLSCASecretRef.DataKey; dataKey != nil {
+			tlsCASecretKey = *dataKey
+		}
+		return "https", &securityConfig{
+			CertFile:       fmt.Sprintf("/var/etcd/ssl/%s/server/tls.crt", tlsTarget),
+			KeyFile:        fmt.Sprintf("/var/etcd/ssl/%s/server/tls.key", tlsTarget),
+			ClientCertAuth: true,
+			TrustedCAFile:  fmt.Sprintf("/var/etcd/ssl/%s/ca/%s", tlsTarget, tlsCASecretKey),
+			AutoTLS:        false,
+		}
 	}
+	return "http", nil
 }

pkg/component/etcd/configmap/configmap_suite_test.go

@@ -21,7 +21,7 @@ import (
 	. "github.com/onsi/gomega"
 )
 
-func TestService(t *testing.T) {
+func TestConfigMap(t *testing.T) {
 	RegisterFailHandler(Fail)
 	RunSpecs(t, "Configmap Component Suite")
 }