Skip to content

operator pdb: fix the remaining issues #7013

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
zhzhuang-zju wants to merge 1 commit into
base: master
Choose a base branch
from
+99 −145
Open

operator pdb: fix the remaining issues #7013

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 8 additions & 19 deletions operator/pkg/controlplane/apiserver/apiserver.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,13 @@ limitations under the License.
package apiserver

import (
"context"
"fmt"

appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/labels"
kuberuntime "k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
clientset "k8s.io/client-go/kubernetes"
clientsetscheme "k8s.io/client-go/kubernetes/scheme"

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

// DeploymentGVK represents the GroupVersionKind (GVK) for a Kubernetes Deployment resource.
var DeploymentGVK = appsv1.SchemeGroupVersion.WithKind("Deployment")

// EnsureKarmadaAPIServer creates karmada apiserver deployment and service resource
func EnsureKarmadaAPIServer(client clientset.Interface, cfg *operatorv1alpha1.KarmadaComponents, name, namespace string, featureGates map[string]bool) error {
if err := installKarmadaAPIServer(client, cfg.KarmadaAPIServer, cfg.Etcd, name, namespace, featureGates); err != nil {
Expand Down Expand Up @@ -88,17 +89,11 @@ func installKarmadaAPIServer(client clientset.Interface, cfg *operatorv1alpha1.K
WithExtraArgs(cfg.ExtraArgs).WithExtraVolumeMounts(cfg.ExtraVolumeMounts).
WithExtraVolumes(cfg.ExtraVolumes).WithSidecarContainers(cfg.SidecarContainers).WithResources(cfg.Resources).ForDeployment(apiserverDeployment)

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

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

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

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

Expand Down
26 changes: 13 additions & 13 deletions operator/pkg/controlplane/apiserver/apiserver_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,9 @@ func TestEnsureKarmadaAPIServer(t *testing.T) {
}

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

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

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

if serviceCount != 1 {
Expand Down Expand Up @@ -135,9 +135,9 @@ func TestEnsureKarmadaAggregatedAPIServer(t *testing.T) {
}

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

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

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

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

// Find the deployment action
Expand Down
14 changes: 3 additions & 11 deletions operator/pkg/controlplane/controlplane.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,19 +17,18 @@ limitations under the License.
package controlplane

import (
"context"
"fmt"

appsv1 "k8s.io/api/apps/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
kuberuntime "k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
clientset "k8s.io/client-go/kubernetes"
clientsetscheme "k8s.io/client-go/kubernetes/scheme"
"k8s.io/klog/v2"

operatorv1alpha1 "github.com/karmada-io/karmada/operator/pkg/apis/operator/v1alpha1"
"github.com/karmada-io/karmada/operator/pkg/constants"
"github.com/karmada-io/karmada/operator/pkg/controlplane/apiserver"
"github.com/karmada-io/karmada/operator/pkg/controlplane/pdb"
"github.com/karmada-io/karmada/operator/pkg/util"
"github.com/karmada-io/karmada/operator/pkg/util/apiclient"
Expand All @@ -49,7 +48,7 @@ func EnsureControlPlaneComponent(component, name, namespace string, featureGates
return nil
}

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

Expand Down Expand Up @@ -83,14 +82,7 @@ func EnsureControlPlaneComponent(component, name, namespace string, featureGates
}

if commonSettings != nil {
// Fetch the persisted Deployment to ensure UID is populated
persisted, err := client.AppsV1().Deployments(namespace).Get(context.TODO(), deployment.GetName(), metav1.GetOptions{})
if err != nil {
return fmt.Errorf("failed to fetch Deployment %s/%s for PDB owner, err: %w", namespace, deployment.GetName(), err)
}
// Build OwnerReference for controller with real UID
gvk := schema.GroupVersionKind{Group: "apps", Version: "v1", Kind: "Deployment"}
ownerRef := *metav1.NewControllerRef(persisted, gvk)
ownerRef := *metav1.NewControllerRef(deployment, apiserver.DeploymentGVK)
if err := pdb.EnsurePodDisruptionBudget(client, pdbName, namespace, commonSettings.PodDisruptionBudgetConfig, deployment.Spec.Template.Labels, []metav1.OwnerReference{ownerRef}); err != nil {
return fmt.Errorf("failed to ensure PDB for component %s (instance: %s), err: %w", component, pdbName, err)
}
Expand Down
11 changes: 5 additions & 6 deletions operator/pkg/controlplane/controlplane_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,8 @@ func TestEnsureAllControlPlaneComponents(t *testing.T) {
}

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

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

if pdbCount != len(components) {
Expand Down
15 changes: 5 additions & 10 deletions operator/pkg/controlplane/etcd/etcd.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,13 @@ limitations under the License.
package etcd

import (
"context"
"fmt"
"strings"

appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
kuberuntime "k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
clientset "k8s.io/client-go/kubernetes"
clientsetscheme "k8s.io/client-go/kubernetes/scheme"
"k8s.io/component-base/cli/flag"
Expand All @@ -38,6 +36,9 @@ import (
"github.com/karmada-io/karmada/operator/pkg/util/patcher"
)

// StatefulSetGVK represents the GroupVersionKind (GVK) for a Kubernetes StatefulSet resource.
var StatefulSetGVK = appsv1.SchemeGroupVersion.WithKind("StatefulSet")

// EnsureKarmadaEtcd creates etcd StatefulSet and service resource.
func EnsureKarmadaEtcd(client clientset.Interface, cfg *operatorv1alpha1.LocalEtcd, name, namespace string) error {
if err := installKarmadaEtcd(client, name, namespace, cfg); err != nil {
Expand Down Expand Up @@ -98,17 +99,11 @@ func installKarmadaEtcd(client clientset.Interface, name, namespace string, cfg
WithPriorityClassName(cfg.CommonSettings.PriorityClassName).
WithVolumeData(cfg.VolumeData).WithResources(cfg.Resources).ForStatefulSet(etcdStatefulSet)

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

// Fetch persisted StatefulSet to get real UID
persisted, err := client.AppsV1().StatefulSets(namespace).Get(context.TODO(), etcdStatefulSet.GetName(), metav1.GetOptions{})
if err != nil {
return fmt.Errorf("failed to fetch StatefulSet %s/%s for PDB owner, err: %w", namespace, etcdStatefulSet.GetName(), err)
}
gvk := schema.GroupVersionKind{Group: "apps", Version: "v1", Kind: "StatefulSet"}
ownerRef := *metav1.NewControllerRef(persisted, gvk)
ownerRef := *metav1.NewControllerRef(etcdStatefulSet, StatefulSetGVK)
if err := pdb.EnsurePodDisruptionBudget(client, util.KarmadaEtcdName(name), namespace, cfg.CommonSettings.PodDisruptionBudgetConfig, etcdStatefulSet.Spec.Template.Labels, []metav1.OwnerReference{ownerRef}); err != nil {
return fmt.Errorf("failed to ensure PDB for etcd component %s, err: %w", util.KarmadaEtcdName(name), err)
}
Expand Down
16 changes: 8 additions & 8 deletions operator/pkg/controlplane/etcd/etcd_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,9 @@ func TestEnsureKarmadaEtcd(t *testing.T) {
}

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

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

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

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

// Find the statefulset action
Expand Down
14 changes: 3 additions & 11 deletions operator/pkg/controlplane/metricsadapter/metricsadapter.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,17 @@ limitations under the License.
package metricsadapter

import (
"context"
"fmt"

appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
kuberuntime "k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
clientset "k8s.io/client-go/kubernetes"
clientsetscheme "k8s.io/client-go/kubernetes/scheme"

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

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

// Fetch persisted Deployment to get real UID
persisted, err := client.AppsV1().Deployments(namespace).Get(context.TODO(), metricAdapter.GetName(), metav1.GetOptions{})
if err != nil {
return fmt.Errorf("failed to fetch Deployment %s/%s for PDB owner, err: %w", namespace, metricAdapter.GetName(), err)
}
// Ensure PDB for the metrics adapter component if configured
gvk := schema.GroupVersionKind{Group: "apps", Version: "v1", Kind: "Deployment"}
ownerRef := *metav1.NewControllerRef(persisted, gvk)
ownerRef := *metav1.NewControllerRef(metricAdapter, apiserver.DeploymentGVK)
if err := pdb.EnsurePodDisruptionBudget(client, util.KarmadaMetricsAdapterName(name), namespace, cfg.CommonSettings.PodDisruptionBudgetConfig, metricAdapter.Spec.Template.Labels, []metav1.OwnerReference{ownerRef}); err != nil {
return fmt.Errorf("failed to ensure PDB for metrics adapter component %s, err: %w", util.KarmadaMetricsAdapterName(name), err)
}
Expand Down
16 changes: 8 additions & 8 deletions operator/pkg/controlplane/metricsadapter/metricsadapter_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,9 @@ func TestEnsureKarmadaMetricAdapter(t *testing.T) {
}

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

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

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

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

// Find the deployment action
Expand Down
Loading
Loading