Skip to content

Commit

Permalink
Address review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
anveshreddy18 committed Nov 26, 2024
1 parent 254b23d commit 8121654
Show file tree
Hide file tree
Showing 6 changed files with 26 additions and 8 deletions.
2 changes: 1 addition & 1 deletion api/v1alpha1/helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ func GetServiceAccountName(etcdObjMeta metav1.ObjectMeta) string {

// GetConfigMapName returns the name of the configmap for the Etcd.
func GetConfigMapName(etcdObjMeta metav1.ObjectMeta) string {
return fmt.Sprintf("%s-config-%s", etcdObjMeta.Name, etcdObjMeta.UID[:6])
return fmt.Sprintf("%s-config", etcdObjMeta.Name)
}

// GetCompactionJobName returns the compaction job name for the Etcd.
Expand Down
2 changes: 1 addition & 1 deletion api/v1alpha1/helper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ func TestGetConfigMapName(t *testing.T) {
uid := uuid.NewUUID()
etcdObjMeta := createEtcdObjectMetadata(uid, nil, nil, false)
configMapName := GetConfigMapName(etcdObjMeta)
g.Expect(configMapName).To(Equal(etcdObjMeta.Name + "-config-" + string(uid[:6])))
g.Expect(configMapName).To(Equal(etcdObjMeta.Name + "-config"))
}

func TestGetCompactionJobName(t *testing.T) {
Expand Down
2 changes: 1 addition & 1 deletion docs/proposals/03-scaling-up-an-etcd-cluster.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ Now, it is detected whether peer URL was TLS enabled or not for single node etcd
- If peer URL was not TLS enabled then etcd-druid has to intervene and make sure peer URL should be TLS enabled first for the single node before marking the cluster for scale-up.

## Action taken by etcd-druid to enable the peerURL TLS
1. Etcd-druid will update the `{etcd.Name}-config-{etcdUID}` config-map with new config like initial-cluster,initial-advertise-peer-urls etc. Backup-restore will detect this change and update the member lease annotation to `member.etcd.gardener.cloud/tls-enabled: "true"`.
1. Etcd-druid will update the `{etcd.Name}-config` config-map with new config like initial-cluster,initial-advertise-peer-urls etc. Backup-restore will detect this change and update the member lease annotation to `member.etcd.gardener.cloud/tls-enabled: "true"`.
2. In case the peer URL TLS has been changed to `enabled`: Etcd-druid will add tasks to the deployment flow:
- Check if peer TLS has been enabled for existing StatefulSet pods, by checking the member leases for the annotation `member.etcd.gardener.cloud/tls-enabled`.
- If peer TLS enablement is pending for any of the members, then check and patch the StatefulSet with the peer TLS volume mounts, if not already patched. This will cause a rolling update of the existing StatefulSet pods, which allows etcd-backup-restore to update the member peer URL in the etcd cluster.
Expand Down
22 changes: 20 additions & 2 deletions internal/component/configmap/configmap_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,7 @@ func matchConfigMap(g *WithT, etcd *druidv1alpha1.Etcd, actualConfigMap corev1.C
err := yaml.Unmarshal([]byte(actualETCDConfigYAML), &actualETCDConfig)
g.Expect(err).ToNot(HaveOccurred())
g.Expect(actualETCDConfig).To(MatchKeys(IgnoreExtras|IgnoreMissing, Keys{
"name": Equal(fmt.Sprintf("etcd-%s", etcd.UID[:6])),
"name": Equal("etcd-config"),
"data-dir": Equal(fmt.Sprintf("%s/new.etcd", common.VolumeMountPathEtcdData)),
"metrics": Equal(string(druidv1alpha1.Basic)),
"snapshot-count": Equal(ptr.Deref(etcd.Spec.Etcd.SnapshotCount, defaultSnapshotCount)),
Expand Down Expand Up @@ -370,8 +370,26 @@ func matchClientTLSRelatedConfiguration(g *WithT, etcd *druidv1alpha1.Etcd, actu
}
}

func assertAdvertiseURLs(etcd *druidv1alpha1.Etcd, advertiseURLType, scheme string) map[string][]string {
var port int32
switch advertiseURLType {
case advertiseURLTypePeer:
port = ptr.Deref(etcd.Spec.Etcd.ServerPort, common.DefaultPortEtcdPeer)
case advertiseURLTypeClient:
port = ptr.Deref(etcd.Spec.Etcd.ClientPort, common.DefaultPortEtcdClient)
default:
return nil
}
advUrlsMap := make(map[string][]string)
for i := 0; i < int(etcd.Spec.Replicas); i++ {
podName := druidv1alpha1.GetOrdinalPodName(etcd.ObjectMeta, i)
advUrlsMap[podName] = []string{fmt.Sprintf("%s://%s.%s.%s.svc:%d", scheme, podName, druidv1alpha1.GetPeerServiceName(etcd.ObjectMeta), etcd.Namespace, port)}
}
return advUrlsMap
}

func convertAdvertiseURLsValuesToInterface(etcd *druidv1alpha1.Etcd, advertiseURLType, scheme string) map[string]interface{} {
advertiseUrlsMap := getAdvertiseURLs(etcd, advertiseURLType, scheme, druidv1alpha1.GetPeerServiceName(etcd.ObjectMeta))
advertiseUrlsMap := assertAdvertiseURLs(etcd, advertiseURLType, scheme)
advertiseUrlsInterface := make(map[string]interface{}, len(advertiseUrlsMap))
for podName, urlList := range advertiseUrlsMap {
urlsListInterface := make([]interface{}, len(urlList))
Expand Down
2 changes: 1 addition & 1 deletion internal/component/configmap/etcdconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ func createEtcdConfig(etcd *druidv1alpha1.Etcd) *etcdConfig {
peerScheme, peerSecurityConfig := getSchemeAndSecurityConfig(etcd.Spec.Etcd.PeerUrlTLS, common.VolumeMountPathEtcdPeerCA, common.VolumeMountPathEtcdPeerServerTLS)
peerSvcName := druidv1alpha1.GetPeerServiceName(etcd.ObjectMeta)
cfg := &etcdConfig{
Name: fmt.Sprintf("etcd-%s", etcd.UID[:6]),
Name: "etcd-config",
DataDir: defaultDataDir,
Metrics: ptr.Deref(etcd.Spec.Etcd.Metrics, druidv1alpha1.Basic),
SnapshotCount: getSnapshotCount(etcd),
Expand Down
4 changes: 2 additions & 2 deletions test/e2e/etcd_backup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ func checkEtcdReady(ctx context.Context, cl client.Client, logger logr.Logger, e

logger.Info("Checking configmap")
cm := &corev1.ConfigMap{}
ExpectWithOffset(2, cl.Get(ctx, client.ObjectKey{Name: etcd.Name + "-config-" + string(etcd.UID[:6]), Namespace: etcd.Namespace}, cm)).To(Succeed())
ExpectWithOffset(2, cl.Get(ctx, client.ObjectKey{Name: etcd.Name + "-config", Namespace: etcd.Namespace}, cm)).To(Succeed())

logger.Info("Checking client service")
svc := &corev1.Service{}
Expand Down Expand Up @@ -280,7 +280,7 @@ func deleteAndCheckEtcd(ctx context.Context, cl client.Client, logger logr.Logge
ExpectWithOffset(1,
cl.Get(
ctx,
client.ObjectKey{Name: etcd.Name + "-config-" + string(etcd.UID[:6]), Namespace: etcd.Namespace},
client.ObjectKey{Name: etcd.Name + "-config", Namespace: etcd.Namespace},
&corev1.ConfigMap{},
),
).Should(matchers.BeNotFoundError())
Expand Down

0 comments on commit 8121654

Please sign in to comment.