Skip to content

Commit b1ac00e

Browse files
zhzhuang-zjuzhzhuang-zju
committed
operator pdb: fix the remaining issues
Signed-off-by: zhzhuang-zju <[email protected]>
1 parent 88b3e4b commit b1ac00e

File tree

15 files changed

+99
-145
lines changed

15 files changed

+99
-145
lines changed

‎operator/pkg/controlplane/apiserver/apiserver.go‎

Lines changed: 8 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -17,15 +17,13 @@ limitations under the License.
1717
package apiserver
1818

1919
import (
20-
"context"
2120
"fmt"
2221

2322
appsv1 "k8s.io/api/apps/v1"
2423
corev1 "k8s.io/api/core/v1"
2524
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2625
"k8s.io/apimachinery/pkg/labels"
2726
kuberuntime "k8s.io/apimachinery/pkg/runtime"
28-
"k8s.io/apimachinery/pkg/runtime/schema"
2927
clientset "k8s.io/client-go/kubernetes"
3028
clientsetscheme "k8s.io/client-go/kubernetes/scheme"
3129

@@ -37,6 +35,9 @@ import (
3735
"github.com/karmada-io/karmada/operator/pkg/util/patcher"
3836
)
3937

38+
// DeploymentGVK represents the GroupVersionKind (GVK) for a Kubernetes Deployment resource.
39+
var DeploymentGVK = appsv1.SchemeGroupVersion.WithKind("Deployment")
40+
4041
// EnsureKarmadaAPIServer creates karmada apiserver deployment and service resource
4142
func EnsureKarmadaAPIServer(client clientset.Interface, cfg *operatorv1alpha1.KarmadaComponents, name, namespace string, featureGates map[string]bool) error {
4243
if err := installKarmadaAPIServer(client, cfg.KarmadaAPIServer, cfg.Etcd, name, namespace, featureGates); err != nil {
@@ -88,17 +89,11 @@ func installKarmadaAPIServer(client clientset.Interface, cfg *operatorv1alpha1.K
8889
WithExtraArgs(cfg.ExtraArgs).WithExtraVolumeMounts(cfg.ExtraVolumeMounts).
8990
WithExtraVolumes(cfg.ExtraVolumes).WithSidecarContainers(cfg.SidecarContainers).WithResources(cfg.Resources).ForDeployment(apiserverDeployment)
9091

91-
if err := apiclient.CreateOrUpdateDeployment(client, apiserverDeployment); err != nil {
92+
if apiserverDeployment, err = apiclient.CreateOrUpdateDeployment(client, apiserverDeployment); err != nil {
9293
return fmt.Errorf("error when creating deployment for %s, err: %w", apiserverDeployment.Name, err)
9394
}
9495

95-
// Fetch persisted Deployment to get real UID
96-
persisted, err := client.AppsV1().Deployments(namespace).Get(context.TODO(), apiserverDeployment.GetName(), metav1.GetOptions{})
97-
if err != nil {
98-
return fmt.Errorf("failed to fetch Deployment %s/%s for PDB owner, err: %w", namespace, apiserverDeployment.GetName(), err)
99-
}
100-
gvk := schema.GroupVersionKind{Group: "apps", Version: "v1", Kind: "Deployment"}
101-
ownerRef := *metav1.NewControllerRef(persisted, gvk)
96+
ownerRef := *metav1.NewControllerRef(apiserverDeployment, DeploymentGVK)
10297
if err := pdb.EnsurePodDisruptionBudget(client, util.KarmadaAPIServerName(name), namespace, cfg.CommonSettings.PodDisruptionBudgetConfig, apiserverDeployment.Spec.Template.Labels, []metav1.OwnerReference{ownerRef}); err != nil {
10398
return fmt.Errorf("failed to ensure PDB for apiserver component %s, err: %w", util.KarmadaAPIServerName(name), err)
10499
}
@@ -171,18 +166,12 @@ func installKarmadaAggregatedAPIServer(client clientset.Interface, cfg *operator
171166
WithPriorityClassName(cfg.CommonSettings.PriorityClassName).
172167
WithExtraArgs(cfg.ExtraArgs).WithFeatureGates(featureGates).WithResources(cfg.Resources).ForDeployment(aggregatedAPIServerDeployment)
173168

174-
if err := apiclient.CreateOrUpdateDeployment(client, aggregatedAPIServerDeployment); err != nil {
169+
if aggregatedAPIServerDeployment, err = apiclient.CreateOrUpdateDeployment(client, aggregatedAPIServerDeployment); err != nil {
175170
return fmt.Errorf("error when creating deployment for %s, err: %w", aggregatedAPIServerDeployment.Name, err)
176171
}
177172

178-
// Fetch persisted Deployment to get real UID
179-
persistedAgg, err := client.AppsV1().Deployments(namespace).Get(context.TODO(), aggregatedAPIServerDeployment.GetName(), metav1.GetOptions{})
180-
if err != nil {
181-
return fmt.Errorf("failed to fetch Deployment %s/%s for PDB owner, err: %w", namespace, aggregatedAPIServerDeployment.GetName(), err)
182-
}
183-
gvk2 := schema.GroupVersionKind{Group: "apps", Version: "v1", Kind: "Deployment"}
184-
ownerRef2 := *metav1.NewControllerRef(persistedAgg, gvk2)
185-
if err := pdb.EnsurePodDisruptionBudget(client, util.KarmadaAggregatedAPIServerName(name), namespace, cfg.CommonSettings.PodDisruptionBudgetConfig, aggregatedAPIServerDeployment.Spec.Template.Labels, []metav1.OwnerReference{ownerRef2}); err != nil {
173+
ownerRef := *metav1.NewControllerRef(aggregatedAPIServerDeployment, DeploymentGVK)
174+
if err := pdb.EnsurePodDisruptionBudget(client, util.KarmadaAggregatedAPIServerName(name), namespace, cfg.CommonSettings.PodDisruptionBudgetConfig, aggregatedAPIServerDeployment.Spec.Template.Labels, []metav1.OwnerReference{ownerRef}); err != nil {
186175
return fmt.Errorf("failed to ensure PDB for aggregated apiserver component %s, err: %w", util.KarmadaAggregatedAPIServerName(name), err)
187176
}
188177

‎operator/pkg/controlplane/apiserver/apiserver_test.go‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -67,9 +67,9 @@ func TestEnsureKarmadaAPIServer(t *testing.T) {
6767
}
6868

6969
actions := fakeClient.Actions()
70-
// We now create deployment, service, PDB, and get deployment, so expect 4 actions
71-
if len(actions) != 4 {
72-
t.Fatalf("expected 4 actions, but got %d", len(actions))
70+
// We now create deployment, service and PDB, so expect 3 actions
71+
if len(actions) != 3 {
72+
t.Fatalf("expected 3 actions, but got %d", len(actions))
7373
}
7474

7575
// Check that we have deployment, service, and PDB
@@ -86,8 +86,8 @@ func TestEnsureKarmadaAPIServer(t *testing.T) {
8686
}
8787
}
8888

89-
if deploymentCount != 2 {
90-
t.Errorf("expected 2 deployment actions (create + get), but got %d", deploymentCount)
89+
if deploymentCount != 1 {
90+
t.Errorf("expected 1 deployment actions, but got %d", deploymentCount)
9191
}
9292

9393
if serviceCount != 1 {
@@ -135,9 +135,9 @@ func TestEnsureKarmadaAggregatedAPIServer(t *testing.T) {
135135
}
136136

137137
actions := fakeClient.Actions()
138-
// We now create deployment, service, PDB, and get deployment, so expect 4 actions
139-
if len(actions) != 4 {
140-
t.Fatalf("expected 4 actions, but got %d", len(actions))
138+
// We now create deployment, service and PDB, so expect 3 actions
139+
if len(actions) != 3 {
140+
t.Fatalf("expected 3 actions, but got %d", len(actions))
141141
}
142142

143143
// Check that we have deployment, service, and PDB
@@ -154,8 +154,8 @@ func TestEnsureKarmadaAggregatedAPIServer(t *testing.T) {
154154
}
155155
}
156156

157-
if deploymentCount != 2 {
158-
t.Errorf("expected 2 deployment actions (create + get), but got %d", deploymentCount)
157+
if deploymentCount != 1 {
158+
t.Errorf("expected 1 deployment actions, but got %d", deploymentCount)
159159
}
160160

161161
if serviceCount != 1 {
@@ -377,9 +377,9 @@ func contains(slice []string, item string) bool {
377377
func verifyDeploymentCreation(client *fakeclientset.Clientset) (*appsv1.Deployment, error) {
378378
// Assert that a Deployment and PDB were created.
379379
actions := client.Actions()
380-
// We now create deployment, PDB, and perform a get action, so expect 3 actions
381-
if len(actions) != 3 {
382-
return nil, fmt.Errorf("expected exactly 3 actions (deployment + PDB + get), but got %d actions", len(actions))
380+
// We now create deployment and PDB, so expect 2 actions
381+
if len(actions) != 2 {
382+
return nil, fmt.Errorf("expected exactly 2 actions (deployment + PDB), but got %d actions", len(actions))
383383
}
384384

385385
// Find the deployment action

‎operator/pkg/controlplane/controlplane.go‎

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -17,19 +17,18 @@ limitations under the License.
1717
package controlplane
1818

1919
import (
20-
"context"
2120
"fmt"
2221

2322
appsv1 "k8s.io/api/apps/v1"
2423
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2524
kuberuntime "k8s.io/apimachinery/pkg/runtime"
26-
"k8s.io/apimachinery/pkg/runtime/schema"
2725
clientset "k8s.io/client-go/kubernetes"
2826
clientsetscheme "k8s.io/client-go/kubernetes/scheme"
2927
"k8s.io/klog/v2"
3028

3129
operatorv1alpha1 "github.com/karmada-io/karmada/operator/pkg/apis/operator/v1alpha1"
3230
"github.com/karmada-io/karmada/operator/pkg/constants"
31+
"github.com/karmada-io/karmada/operator/pkg/controlplane/apiserver"
3332
"github.com/karmada-io/karmada/operator/pkg/controlplane/pdb"
3433
"github.com/karmada-io/karmada/operator/pkg/util"
3534
"github.com/karmada-io/karmada/operator/pkg/util/apiclient"
@@ -49,7 +48,7 @@ func EnsureControlPlaneComponent(component, name, namespace string, featureGates
4948
return nil
5049
}
5150

52-
if err := apiclient.CreateOrUpdateDeployment(client, deployment); err != nil {
51+
if deployment, err = apiclient.CreateOrUpdateDeployment(client, deployment); err != nil {
5352
return fmt.Errorf("failed to create deployment resource for component %s, err: %w", component, err)
5453
}
5554

@@ -83,14 +82,7 @@ func EnsureControlPlaneComponent(component, name, namespace string, featureGates
8382
}
8483

8584
if commonSettings != nil {
86-
// Fetch the persisted Deployment to ensure UID is populated
87-
persisted, err := client.AppsV1().Deployments(namespace).Get(context.TODO(), deployment.GetName(), metav1.GetOptions{})
88-
if err != nil {
89-
return fmt.Errorf("failed to fetch Deployment %s/%s for PDB owner, err: %w", namespace, deployment.GetName(), err)
90-
}
91-
// Build OwnerReference for controller with real UID
92-
gvk := schema.GroupVersionKind{Group: "apps", Version: "v1", Kind: "Deployment"}
93-
ownerRef := *metav1.NewControllerRef(persisted, gvk)
85+
ownerRef := *metav1.NewControllerRef(deployment, apiserver.DeploymentGVK)
9486
if err := pdb.EnsurePodDisruptionBudget(client, pdbName, namespace, commonSettings.PodDisruptionBudgetConfig, deployment.Spec.Template.Labels, []metav1.OwnerReference{ownerRef}); err != nil {
9587
return fmt.Errorf("failed to ensure PDB for component %s (instance: %s), err: %w", component, pdbName, err)
9688
}

‎operator/pkg/controlplane/controlplane_test.go‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -110,8 +110,8 @@ func TestEnsureAllControlPlaneComponents(t *testing.T) {
110110
}
111111

112112
actions := fakeClient.Actions()
113-
// We now create both deployments and PDBs,include get the deployment. so expect 3 actions per component
114-
expectedActions := len(components) * 3
113+
// We now create both deployments and PDBs. so expect 2 actions per component
114+
expectedActions := len(components) * 2
115115
if len(actions) != expectedActions {
116116
t.Fatalf("expected %d actions, but got %d", expectedActions, len(actions))
117117
}
@@ -127,10 +127,9 @@ func TestEnsureAllControlPlaneComponents(t *testing.T) {
127127
}
128128
}
129129

130-
// Each component has 2 deployment actions (create + get) and 1 PDB action
131-
expectedDeployments := len(components) * 2
132-
if deploymentCount != expectedDeployments {
133-
t.Errorf("expected %d deployment actions (create + get for each component), but got %d", expectedDeployments, deploymentCount)
130+
// Each component has 1 deployment actions and 1 PDB action
131+
if deploymentCount != len(components) {
132+
t.Errorf("expected %d deployment actions (create + get for each component), but got %d", len(components), deploymentCount)
134133
}
135134

136135
if pdbCount != len(components) {

‎operator/pkg/controlplane/etcd/etcd.go‎

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -17,15 +17,13 @@ limitations under the License.
1717
package etcd
1818

1919
import (
20-
"context"
2120
"fmt"
2221
"strings"
2322

2423
appsv1 "k8s.io/api/apps/v1"
2524
corev1 "k8s.io/api/core/v1"
2625
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2726
kuberuntime "k8s.io/apimachinery/pkg/runtime"
28-
"k8s.io/apimachinery/pkg/runtime/schema"
2927
clientset "k8s.io/client-go/kubernetes"
3028
clientsetscheme "k8s.io/client-go/kubernetes/scheme"
3129
"k8s.io/component-base/cli/flag"
@@ -38,6 +36,9 @@ import (
3836
"github.com/karmada-io/karmada/operator/pkg/util/patcher"
3937
)
4038

39+
// StatefulSetGVK represents the GroupVersionKind (GVK) for a Kubernetes StatefulSet resource.
40+
var StatefulSetGVK = appsv1.SchemeGroupVersion.WithKind("StatefulSet")
41+
4142
// EnsureKarmadaEtcd creates etcd StatefulSet and service resource.
4243
func EnsureKarmadaEtcd(client clientset.Interface, cfg *operatorv1alpha1.LocalEtcd, name, namespace string) error {
4344
if err := installKarmadaEtcd(client, name, namespace, cfg); err != nil {
@@ -98,17 +99,11 @@ func installKarmadaEtcd(client clientset.Interface, name, namespace string, cfg
9899
WithPriorityClassName(cfg.CommonSettings.PriorityClassName).
99100
WithVolumeData(cfg.VolumeData).WithResources(cfg.Resources).ForStatefulSet(etcdStatefulSet)
100101

101-
if err := apiclient.CreateOrUpdateStatefulSet(client, etcdStatefulSet); err != nil {
102+
if etcdStatefulSet, err = apiclient.CreateOrUpdateStatefulSet(client, etcdStatefulSet); err != nil {
102103
return fmt.Errorf("error when creating Etcd statefulset, err: %w", err)
103104
}
104105

105-
// Fetch persisted StatefulSet to get real UID
106-
persisted, err := client.AppsV1().StatefulSets(namespace).Get(context.TODO(), etcdStatefulSet.GetName(), metav1.GetOptions{})
107-
if err != nil {
108-
return fmt.Errorf("failed to fetch StatefulSet %s/%s for PDB owner, err: %w", namespace, etcdStatefulSet.GetName(), err)
109-
}
110-
gvk := schema.GroupVersionKind{Group: "apps", Version: "v1", Kind: "StatefulSet"}
111-
ownerRef := *metav1.NewControllerRef(persisted, gvk)
106+
ownerRef := *metav1.NewControllerRef(etcdStatefulSet, StatefulSetGVK)
112107
if err := pdb.EnsurePodDisruptionBudget(client, util.KarmadaEtcdName(name), namespace, cfg.CommonSettings.PodDisruptionBudgetConfig, etcdStatefulSet.Spec.Template.Labels, []metav1.OwnerReference{ownerRef}); err != nil {
113108
return fmt.Errorf("failed to ensure PDB for etcd component %s, err: %w", util.KarmadaEtcdName(name), err)
114109
}

‎operator/pkg/controlplane/etcd/etcd_test.go‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -66,9 +66,9 @@ func TestEnsureKarmadaEtcd(t *testing.T) {
6666
}
6767

6868
actions := fakeClient.Actions()
69-
// We now create statefulset, 2 services (peer + client), and PDB, include get statefulset, so expect 5 actions
70-
if len(actions) != 5 {
71-
t.Fatalf("expected 5 actions, but got %d", len(actions))
69+
// We now create statefulset, 2 services (peer + client), and PDB, so expect 4 actions
70+
if len(actions) != 4 {
71+
t.Fatalf("expected 4 actions, but got %d", len(actions))
7272
}
7373

7474
// Check that we have statefulset, 2 services, and PDB
@@ -85,8 +85,8 @@ func TestEnsureKarmadaEtcd(t *testing.T) {
8585
}
8686
}
8787

88-
if statefulsetCount != 2 {
89-
t.Errorf("expected 2 statefulset actions (create + get), but got %d", statefulsetCount)
88+
if statefulsetCount != 1 {
89+
t.Errorf("expected 1 statefulset actions, but got %d", statefulsetCount)
9090
}
9191

9292
if serviceCount != 2 {
@@ -147,9 +147,9 @@ func TestInstallKarmadaEtcd(t *testing.T) {
147147
func verifyStatefulSetCreation(client *fakeclientset.Clientset) (*appsv1.StatefulSet, error) {
148148
// Assert that a StatefulSet and PDB were created.
149149
actions := client.Actions()
150-
// We now create statefulset, PDB, and perform a get action, so expect 3 actions
151-
if len(actions) != 3 {
152-
return nil, fmt.Errorf("expected exactly 3 actions (statefulset + PDB + get), but got %d actions", len(actions))
150+
// We now create statefulset and PDB, so expect 2 actions
151+
if len(actions) != 2 {
152+
return nil, fmt.Errorf("expected exactly 2 actions (statefulset + PDB), but got %d actions", len(actions))
153153
}
154154

155155
// Find the statefulset action

‎operator/pkg/controlplane/metricsadapter/metricsadapter.go‎

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -17,18 +17,17 @@ limitations under the License.
1717
package metricsadapter
1818

1919
import (
20-
"context"
2120
"fmt"
2221

2322
appsv1 "k8s.io/api/apps/v1"
2423
corev1 "k8s.io/api/core/v1"
2524
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2625
kuberuntime "k8s.io/apimachinery/pkg/runtime"
27-
"k8s.io/apimachinery/pkg/runtime/schema"
2826
clientset "k8s.io/client-go/kubernetes"
2927
clientsetscheme "k8s.io/client-go/kubernetes/scheme"
3028

3129
operatorv1alpha1 "github.com/karmada-io/karmada/operator/pkg/apis/operator/v1alpha1"
30+
"github.com/karmada-io/karmada/operator/pkg/controlplane/apiserver"
3231
"github.com/karmada-io/karmada/operator/pkg/controlplane/pdb"
3332
"github.com/karmada-io/karmada/operator/pkg/util"
3433
"github.com/karmada-io/karmada/operator/pkg/util/apiclient"
@@ -71,18 +70,11 @@ func installKarmadaMetricAdapter(client clientset.Interface, cfg *operatorv1alph
7170
patcher.NewPatcher().WithAnnotations(cfg.Annotations).WithLabels(cfg.Labels).WithPriorityClassName(cfg.CommonSettings.PriorityClassName).
7271
WithResources(cfg.Resources).ForDeployment(metricAdapter)
7372

74-
if err := apiclient.CreateOrUpdateDeployment(client, metricAdapter); err != nil {
73+
if metricAdapter, err = apiclient.CreateOrUpdateDeployment(client, metricAdapter); err != nil {
7574
return fmt.Errorf("error when creating deployment for %s, err: %w", metricAdapter.Name, err)
7675
}
7776

78-
// Fetch persisted Deployment to get real UID
79-
persisted, err := client.AppsV1().Deployments(namespace).Get(context.TODO(), metricAdapter.GetName(), metav1.GetOptions{})
80-
if err != nil {
81-
return fmt.Errorf("failed to fetch Deployment %s/%s for PDB owner, err: %w", namespace, metricAdapter.GetName(), err)
82-
}
83-
// Ensure PDB for the metrics adapter component if configured
84-
gvk := schema.GroupVersionKind{Group: "apps", Version: "v1", Kind: "Deployment"}
85-
ownerRef := *metav1.NewControllerRef(persisted, gvk)
77+
ownerRef := *metav1.NewControllerRef(metricAdapter, apiserver.DeploymentGVK)
8678
if err := pdb.EnsurePodDisruptionBudget(client, util.KarmadaMetricsAdapterName(name), namespace, cfg.CommonSettings.PodDisruptionBudgetConfig, metricAdapter.Spec.Template.Labels, []metav1.OwnerReference{ownerRef}); err != nil {
8779
return fmt.Errorf("failed to ensure PDB for metrics adapter component %s, err: %w", util.KarmadaMetricsAdapterName(name), err)
8880
}

‎operator/pkg/controlplane/metricsadapter/metricsadapter_test.go‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -64,9 +64,9 @@ func TestEnsureKarmadaMetricAdapter(t *testing.T) {
6464
}
6565

6666
actions := fakeClient.Actions()
67-
// We now create deployment, service, PDB, and perform a get action, so expect 4 actions
68-
if len(actions) != 4 {
69-
t.Fatalf("expected 4 actions, but got %d", len(actions))
67+
// We now create deployment, service and PDB, so expect 4 actions
68+
if len(actions) != 3 {
69+
t.Fatalf("expected 3 actions, but got %d", len(actions))
7070
}
7171

7272
// Check that we have deployment, service, and PDB
@@ -83,8 +83,8 @@ func TestEnsureKarmadaMetricAdapter(t *testing.T) {
8383
}
8484
}
8585

86-
if deploymentCount != 2 {
87-
t.Errorf("expected 2 deployment actions (create + get), but got %d", deploymentCount)
86+
if deploymentCount != 1 {
87+
t.Errorf("expected 1 deployment actions, but got %d", deploymentCount)
8888
}
8989

9090
if serviceCount != 1 {
@@ -180,9 +180,9 @@ func TestCreateKarmadaMetricAdapterService(t *testing.T) {
180180
func verifyDeploymentCreation(client *fakeclientset.Clientset) (*appsv1.Deployment, error) {
181181
// Assert that a Deployment and PDB were created.
182182
actions := client.Actions()
183-
// We now create deployment, PDB, and perform a get action, so expect 3 actions
184-
if len(actions) != 3 {
185-
return nil, fmt.Errorf("expected exactly 3 actions (deployment + PDB + get), but got %d actions", len(actions))
183+
// We now create deployment and PDB, so expect 2 actions
184+
if len(actions) != 2 {
185+
return nil, fmt.Errorf("expected exactly 2 actions (deployment + PDB), but got %d actions", len(actions))
186186
}
187187

188188
// Find the deployment action

0 commit comments

Comments
 (0)