Skip to content

Commit

Permalink
Addressed review comment from Madhav about OwnerReferences.
Browse files Browse the repository at this point in the history
This commit addresses the review comment gardener#539 (comment)
  • Loading branch information
seshachalam-yv committed May 31, 2023
1 parent 40fe384 commit 3a879d0
Show file tree
Hide file tree
Showing 39 changed files with 108 additions and 128 deletions.
4 changes: 2 additions & 2 deletions api/v1alpha1/types_etcd.go
Original file line number Diff line number Diff line change
Expand Up @@ -460,8 +460,8 @@ func (e *Etcd) GetDefaultLabels() map[string]string {
}

// GetAsOwnerReference returns an OwnerReference object that represents the current Etcd instance.
func (e *Etcd) GetAsOwnerReference() metav1.OwnerReference {
return metav1.OwnerReference{
func (e *Etcd) GetAsOwnerReference() *metav1.OwnerReference {
return &metav1.OwnerReference{
APIVersion: GroupVersion.String(),
Kind: "Etcd",
Name: e.Name,
Expand Down
5 changes: 3 additions & 2 deletions api/v1alpha1/types_etcd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -138,7 +137,9 @@ var _ = Describe("Etcd", func() {
Controller: pointer.Bool(true),
BlockOwnerDeletion: pointer.Bool(true),
}
Expect(created.GetAsOwnerReference()).To(Equal(expected))
actual := created.GetAsOwnerReference()
Expect(actual).NotTo(BeNil())
Expect(*created.GetAsOwnerReference()).To(Equal(expected))
})
})

Expand Down
2 changes: 1 addition & 1 deletion pkg/component/etcd/configmap/configmap.go
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ func (c *component) syncConfigmap(ctx context.Context, cm *corev1.ConfigMap) err
cm.Name = c.values.Name
cm.Namespace = c.namespace
cm.Labels = c.values.Labels
cm.OwnerReferences = c.values.OwnerReferences
cm.OwnerReferences = []metav1.OwnerReference{*c.values.OwnerReference}
cm.Data = map[string]string{"etcd.conf.yaml": c.getEtcdConfigYaml()}
return nil
})
Expand Down
2 changes: 1 addition & 1 deletion pkg/component/etcd/configmap/configmap_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ var _ = Describe("Configmap", func() {
func checkConfigmap(cm *corev1.ConfigMap, values *Values, namespace string) {

Expect(cm.Name).To(Equal(values.Name))
Expect(cm.OwnerReferences).To(Equal(values.OwnerReferences))
Expect(cm.OwnerReferences).To(Equal([]metav1.OwnerReference{*values.OwnerReference}))
Expect(cm.Labels).To(Equal(values.Labels))
configYML := cm.Data[etcdConfig]
config := map[string]interface{}{}
Expand Down
4 changes: 2 additions & 2 deletions pkg/component/etcd/configmap/values.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,6 @@ type Values struct {
ConfigMapChecksum string
// Labels is the labels of deployed configmap
Labels map[string]string
// OwnerReferences are the OwnerReferences of the Configmap.
OwnerReferences []metav1.OwnerReference
// OwnerReference are the OwnerReference of the Configmap.
OwnerReference *metav1.OwnerReference
}
3 changes: 1 addition & 2 deletions pkg/component/etcd/configmap/values_helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import (
"strings"

druidv1alpha1 "github.com/gardener/etcd-druid/api/v1alpha1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/utils/pointer"
)

Expand All @@ -41,7 +40,7 @@ func GenerateValues(etcd *druidv1alpha1.Etcd) *Values {
ServerPort: etcd.Spec.Etcd.ServerPort,
AutoCompactionMode: etcd.Spec.Common.AutoCompactionMode,
AutoCompactionRetention: etcd.Spec.Common.AutoCompactionRetention,
OwnerReferences: []metav1.OwnerReference{etcd.GetAsOwnerReference()},
OwnerReference: etcd.GetAsOwnerReference(),
Labels: etcd.GetDefaultLabels(),
}
return values
Expand Down
3 changes: 2 additions & 1 deletion pkg/component/etcd/lease/lease_member.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"github.com/gardener/gardener/pkg/controllerutils"
"github.com/gardener/gardener/pkg/utils/flow"
coordinationv1 "k8s.io/api/coordination/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/sets"
"sigs.k8s.io/controller-runtime/pkg/client"
)
Expand Down Expand Up @@ -55,7 +56,7 @@ func (c *component) syncMemberLeases(ctx context.Context) error {
for k, v := range labels {
lease.Labels[k] = v
}
lease.OwnerReferences = c.values.OwnerReferences
lease.OwnerReferences = []metav1.OwnerReference{*c.values.OwnerReference}
return nil
})
return err
Expand Down
3 changes: 2 additions & 1 deletion pkg/component/etcd/lease/lease_snapshot.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"github.com/gardener/gardener/pkg/controllerutils"

coordinationv1 "k8s.io/api/coordination/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"sigs.k8s.io/controller-runtime/pkg/client"
)

Expand All @@ -32,7 +33,7 @@ func (c *component) syncSnapshotLease(ctx context.Context, lease *coordinationv1
return c.deleteSnapshotLease(ctx, lease)
}
_, err := controllerutils.GetAndCreateOrMergePatch(ctx, c.client, lease, func() error {
lease.OwnerReferences = c.values.OwnerReferences
lease.OwnerReferences = []metav1.OwnerReference{*c.values.OwnerReference}
return nil
})
return err
Expand Down
4 changes: 2 additions & 2 deletions pkg/component/etcd/lease/values.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,6 @@ type Values struct {
Replicas int32
// Labels is the labels of deployed configmap
Labels map[string]string
// OwnerReferences are the OwnerReferences of the Configmap.
OwnerReferences []metav1.OwnerReference
// OwnerReference are the OwnerReference of the Configmap.
OwnerReference *metav1.OwnerReference
}
3 changes: 1 addition & 2 deletions pkg/component/etcd/lease/values_helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ package lease

import (
druidv1alpha1 "github.com/gardener/etcd-druid/api/v1alpha1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

// GenerateValues generates `lease.Values` for the lease component
Expand All @@ -27,6 +26,6 @@ func GenerateValues(etcd *druidv1alpha1.Etcd) Values {
DeltaSnapshotLeaseName: etcd.GetDeltaSnapshotLeaseName(),
FullSnapshotLeaseName: etcd.GetFullSnapshotLeaseName(),
Replicas: etcd.Spec.Replicas,
OwnerReferences: []metav1.OwnerReference{etcd.GetAsOwnerReference()},
OwnerReference: etcd.GetAsOwnerReference(),
}
}
4 changes: 2 additions & 2 deletions pkg/component/etcd/poddisruptionbudget/poddisruptionbudget.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ func (c *component) syncPodDisruptionBudget(ctx context.Context, pdb client.Obje
case *policyv1.PodDisruptionBudget:
_, err := controllerutils.GetAndCreateOrMergePatch(ctx, c.client, pdb, func() error {
pdb.Labels = c.values.Labels
pdb.OwnerReferences = c.values.OwnerReferences
pdb.OwnerReferences = []metav1.OwnerReference{*c.values.OwnerReference}
pdb.Spec.MinAvailable = &intstr.IntOrString{
IntVal: c.values.MinAvailable,
Type: intstr.Int,
Expand All @@ -105,7 +105,7 @@ func (c *component) syncPodDisruptionBudget(ctx context.Context, pdb client.Obje
_, err := controllerutils.GetAndCreateOrMergePatch(ctx, c.client, pdb, func() error {
pdb.Annotations = c.values.Annotations
pdb.Labels = c.values.Labels
pdb.OwnerReferences = c.values.OwnerReferences
pdb.OwnerReferences = []metav1.OwnerReference{*c.values.OwnerReference}
pdb.Spec.MinAvailable = &intstr.IntOrString{
IntVal: c.values.MinAvailable,
Type: intstr.Int,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,7 @@ func checkV1PDB(pdb *policyv1.PodDisruptionBudget, values *Values, expectedNames
func checkPDBMetadata(meta *metav1.ObjectMeta, values *Values, expectedNamespace string) {
Expect(meta.Name).To(Equal(values.Name))
Expect(meta.Namespace).To(Equal(expectedNamespace))
Expect(meta.OwnerReferences).To(Equal(values.OwnerReferences))
Expect(meta.OwnerReferences).To(Equal([]metav1.OwnerReference{*values.OwnerReference}))
Expect(meta.Labels).To(Equal(pdbLabels(values)))
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/component/etcd/poddisruptionbudget/values.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,6 @@ type Values struct {
Annotations map[string]string
// MinAvailable defined the minimum number of pods to be available at any point of time
MinAvailable int32
// OwnerReferences are the OwnerReferences of the PodDisruptionBudget.
OwnerReferences []metav1.OwnerReference
// OwnerReference are the OwnerReference of the PodDisruptionBudget.
OwnerReference *metav1.OwnerReference
}
1 change: 1 addition & 0 deletions pkg/component/etcd/poddisruptionbudget/values_helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ func GenerateValues(etcd *druidv1alpha1.Etcd) Values {
SelectorLabels: etcd.GetDefaultLabels(),
Annotations: annotations,
MinAvailable: int32(CalculatePDBMinAvailable(etcd)),
OwnerReference: etcd.GetAsOwnerReference(),
}
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/component/etcd/role/role.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func (c component) Deploy(ctx context.Context) error {
role.Name = c.values.Name
role.Namespace = c.values.Namespace
role.Labels = c.values.Labels
role.OwnerReferences = c.values.OwnerReferences
role.OwnerReferences = []metav1.OwnerReference{*c.values.OwnerReference}
role.Rules = c.values.Rules
return nil
})
Expand Down
18 changes: 8 additions & 10 deletions pkg/component/etcd/role/role_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ func verifyRoleValues(expected *rbacv1.Role, values *role.Values) {
Expect(expected.Name).To(Equal(values.Name))
Expect(expected.Labels).To(Equal(values.Labels))
Expect(expected.Namespace).To(Equal(values.Namespace))
Expect(expected.OwnerReferences).To(Equal(values.OwnerReferences))
Expect(expected.OwnerReferences).To(Equal([]metav1.OwnerReference{*values.OwnerReference}))
Expect(expected.Rules).To(MatchAllElements(testutils.RuleIterator, Elements{
"coordination.k8s.io": MatchFields(IgnoreExtras, Fields{
"APIGroups": MatchAllElements(testutils.StringArrayIterator, Elements{
Expand Down Expand Up @@ -187,15 +187,13 @@ func getTestRoleValues() *role.Values {
Verbs: []string{"get", "list", "watch"},
},
},
OwnerReferences: []metav1.OwnerReference{
{
APIVersion: v1alpha1.GroupVersion.String(),
Kind: "etcd",
Name: "test-etcd",
UID: "123-456-789",
Controller: pointer.Bool(true),
BlockOwnerDeletion: pointer.Bool(true),
},
OwnerReference: &metav1.OwnerReference{
APIVersion: v1alpha1.GroupVersion.String(),
Kind: "etcd",
Name: "test-etcd",
UID: "123-456-789",
Controller: pointer.Bool(true),
BlockOwnerDeletion: pointer.Bool(true),
},
}
}
4 changes: 2 additions & 2 deletions pkg/component/etcd/role/values.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ type Values struct {
Namespace string
// Rules holds all the PolicyRules for this Role
Rules []rbacv1.PolicyRule
// OwnerReferences are the OwnerReferences of the Role.
OwnerReferences []metav1.OwnerReference
// OwnerReference are the OwnerReference of the Role.
OwnerReference *metav1.OwnerReference
// Labels are the labels of the Role.
Labels map[string]string
}
11 changes: 4 additions & 7 deletions pkg/component/etcd/role/values_helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,15 @@ package role
import (
druidv1alpha1 "github.com/gardener/etcd-druid/api/v1alpha1"
rbacv1 "k8s.io/api/rbac/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

// GenerateValues generates `role.Values` for the role component with the given `etcd` object.
func GenerateValues(etcd *druidv1alpha1.Etcd) *Values {
return &Values{
Name: etcd.GetRoleName(),
Namespace: etcd.Namespace,
Labels: etcd.GetDefaultLabels(),
OwnerReferences: []metav1.OwnerReference{
etcd.GetAsOwnerReference(),
},
Name: etcd.GetRoleName(),
Namespace: etcd.Namespace,
Labels: etcd.GetDefaultLabels(),
OwnerReference: etcd.GetAsOwnerReference(),
Rules: []rbacv1.PolicyRule{
{
APIGroups: []string{"coordination.k8s.io"},
Expand Down
4 changes: 1 addition & 3 deletions pkg/component/etcd/role/values_helper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,7 @@ var _ = Describe("Role", func() {
"name": "etcd",
"instance": etcd.Name,
},
OwnerReferences: []metav1.OwnerReference{
etcd.GetAsOwnerReference(),
},
OwnerReference: etcd.GetAsOwnerReference(),
Rules: []rbacv1.PolicyRule{
{
APIGroups: []string{"coordination.k8s.io"},
Expand Down
2 changes: 1 addition & 1 deletion pkg/component/etcd/rolebinding/rolebinding.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func (c component) Deploy(ctx context.Context) error {
roleBinding.Name = c.values.Name
roleBinding.Namespace = c.values.Namespace
roleBinding.Labels = c.values.Labels
roleBinding.OwnerReferences = c.values.OwnerReferences
roleBinding.OwnerReferences = []metav1.OwnerReference{*c.values.OwnerReference}
roleBinding.RoleRef = rbacv1.RoleRef{
APIGroup: "rbac.authorization.k8s.io",
Kind: "Role",
Expand Down
18 changes: 8 additions & 10 deletions pkg/component/etcd/rolebinding/rolebinding_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ func verifyRoleBindingValues(expected *rbacv1.RoleBinding, values *rolebinding.V
Expect(expected.Labels).To(Equal(values.Labels))
Expect(expected.Namespace).To(Equal(values.Namespace))
Expect(expected.Namespace).To(Equal(values.Namespace))
Expect(expected.OwnerReferences).To(Equal(values.OwnerReferences))
Expect(expected.OwnerReferences).To(Equal([]metav1.OwnerReference{*values.OwnerReference}))
Expect(expected.Subjects).To(Equal([]rbacv1.Subject{
{
Kind: "ServiceAccount",
Expand All @@ -163,15 +163,13 @@ func getTestRoleBindingValues() *rolebinding.Values {
},
RoleName: "test-role",
ServiceAccountName: "test-serviceaccount",
OwnerReferences: []metav1.OwnerReference{
{
APIVersion: v1alpha1.GroupVersion.String(),
Kind: "etcd",
Name: "test-etcd",
UID: "123-456-789",
Controller: pointer.Bool(true),
BlockOwnerDeletion: pointer.Bool(true),
},
OwnerReference: &metav1.OwnerReference{
APIVersion: v1alpha1.GroupVersion.String(),
Kind: "etcd",
Name: "test-etcd",
UID: "123-456-789",
Controller: pointer.Bool(true),
BlockOwnerDeletion: pointer.Bool(true),
},
}
}
4 changes: 2 additions & 2 deletions pkg/component/etcd/rolebinding/values.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ type Values struct {
// ServiceAccountName is the service account subject name for the RoleBinding.
// It is assumed that the ServiceAccount exists in the namespace where the etcd custom resource is created.
ServiceAccountName string
// OwnerReferences are the OwnerReferences of the RoleBinding.
OwnerReferences []metav1.OwnerReference
// OwnerReference are the OwnerReference of the RoleBinding.
OwnerReference *metav1.OwnerReference
// Labels are the labels of the RoleBinding.
Labels map[string]string
}
11 changes: 4 additions & 7 deletions pkg/component/etcd/rolebinding/values_helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,15 @@ package rolebinding

import (
druidv1alpha1 "github.com/gardener/etcd-druid/api/v1alpha1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

// GenerateValues generates `serviceaccount.Values` for the serviceaccount component with the given `etcd` object.
func GenerateValues(etcd *druidv1alpha1.Etcd) *Values {
return &Values{
Name: etcd.GetRoleBindingName(),
Namespace: etcd.Namespace,
Labels: etcd.GetDefaultLabels(),
OwnerReferences: []metav1.OwnerReference{
etcd.GetAsOwnerReference(),
},
Name: etcd.GetRoleBindingName(),
Namespace: etcd.Namespace,
Labels: etcd.GetDefaultLabels(),
OwnerReference: etcd.GetAsOwnerReference(),
RoleName: etcd.GetRoleName(),
ServiceAccountName: etcd.GetServiceAccountName(),
}
Expand Down
4 changes: 1 addition & 3 deletions pkg/component/etcd/rolebinding/values_helper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,7 @@ var _ = Describe("RoleBinding", func() {
},
RoleName: etcd.GetRoleName(),
ServiceAccountName: etcd.GetServiceAccountName(),
OwnerReferences: []metav1.OwnerReference{
etcd.GetAsOwnerReference(),
},
OwnerReference: etcd.GetAsOwnerReference(),
}
)

Expand Down
3 changes: 2 additions & 1 deletion pkg/component/etcd/service/service_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,15 @@ import (
"github.com/gardener/etcd-druid/pkg/utils"
"github.com/gardener/gardener/pkg/controllerutils"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/intstr"
)

func (c *component) syncClientService(ctx context.Context, svc *corev1.Service) error {
_, err := controllerutils.GetAndCreateOrStrategicMergePatch(ctx, c.client, svc, func() error {
svc.Labels = utils.MergeStringMaps(c.values.Labels, c.values.ClientServiceLabels)
svc.Annotations = c.values.ClientServiceAnnotations
svc.OwnerReferences = c.values.OwnerReferences
svc.OwnerReferences = []metav1.OwnerReference{*c.values.OwnerReference}
svc.Spec.Type = corev1.ServiceTypeClusterIP
svc.Spec.SessionAffinity = corev1.ServiceAffinityNone
svc.Spec.Selector = c.values.SelectorLabels
Expand Down
3 changes: 2 additions & 1 deletion pkg/component/etcd/service/service_peer.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,14 @@ import (

"github.com/gardener/gardener/pkg/controllerutils"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/intstr"
)

func (c *component) syncPeerService(ctx context.Context, svc *corev1.Service) error {
_, err := controllerutils.GetAndCreateOrStrategicMergePatch(ctx, c.client, svc, func() error {
svc.Labels = c.values.Labels
svc.OwnerReferences = c.values.OwnerReferences
svc.OwnerReferences = []metav1.OwnerReference{*c.values.OwnerReference}
svc.Spec.Type = corev1.ServiceTypeClusterIP
svc.Spec.ClusterIP = corev1.ClusterIPNone
svc.Spec.SessionAffinity = corev1.ServiceAffinityNone
Expand Down
4 changes: 2 additions & 2 deletions pkg/component/etcd/service/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ var _ = Describe("Service", func() {
})

func checkClientService(svc *corev1.Service, values Values) {
Expect(svc.OwnerReferences).To(Equal(values.OwnerReferences))
Expect(svc.OwnerReferences).To(Equal([]metav1.OwnerReference{*values.OwnerReference}))
Expect(svc.Labels).To(Equal(utils.MergeStringMaps(values.Labels, values.ClientServiceLabels)))
Expect(svc.Spec.Selector).To(Equal(values.SelectorLabels))
Expect(svc.Spec.Type).To(Equal(corev1.ServiceType("ClusterIP")))
Expand All @@ -193,7 +193,7 @@ func checkClientService(svc *corev1.Service, values Values) {
}

func checkPeerService(svc *corev1.Service, values Values) {
Expect(svc.OwnerReferences).To(Equal(values.OwnerReferences))
Expect(svc.OwnerReferences).To(Equal([]metav1.OwnerReference{*values.OwnerReference}))
Expect(svc.Labels).To(Equal(values.Labels))
Expect(svc.Spec.PublishNotReadyAddresses).To(BeTrue())
Expect(svc.Spec.Type).To(Equal(corev1.ServiceType("ClusterIP")))
Expand Down
Loading

0 comments on commit 3a879d0

Please sign in to comment.