Fix: fix webhook deployment yad override logic (#1851)

Co-authored-by: luchen <[email protected]>

pkg/yurtmanager/controller/yurtappdaemon/workloadcontroller/deployment_controller.go

@@ -58,7 +58,7 @@ func (d *DeploymentControllor) DeleteWorkload(yda *v1alpha1.YurtAppDaemon, load
 }
 
 // ApplyTemplate updates the object to the latest revision, depending on the YurtAppDaemon.
-func (d *DeploymentControllor) applyTemplate(scheme *runtime.Scheme, yad *v1alpha1.YurtAppDaemon, nodepool v1alpha1.NodePool, revision string, set *appsv1.Deployment) error {
+func (d *DeploymentControllor) ApplyTemplate(scheme *runtime.Scheme, yad *v1alpha1.YurtAppDaemon, nodepool v1alpha1.NodePool, revision string, set *appsv1.Deployment) error {
 
 	if set.Labels == nil {
 		set.Labels = map[string]string{}
@@ -129,7 +129,7 @@ func (d *DeploymentControllor) UpdateWorkload(load *Workload, yad *v1alpha1.Yurt
 			return getError
 		}
 
-		if err := d.applyTemplate(d.Scheme, yad, nodepool, revision, deploy); err != nil {
+		if err := d.ApplyTemplate(d.Scheme, yad, nodepool, revision, deploy); err != nil {
 			return err
 		}
 		updateError = d.Client.Update(context.TODO(), deploy)
@@ -145,7 +145,7 @@ func (d *DeploymentControllor) CreateWorkload(yad *v1alpha1.YurtAppDaemon, nodep
 	klog.Infof("YurtAppDaemon[%s/%s] prepare create new deployment by nodepool %s ", yad.GetNamespace(), yad.GetName(), nodepool.GetName())
 
 	deploy := appsv1.Deployment{}
-	if err := d.applyTemplate(d.Scheme, yad, nodepool, revision, &deploy); err != nil {
+	if err := d.ApplyTemplate(d.Scheme, yad, nodepool, revision, &deploy); err != nil {
 		klog.Errorf("YurtAppDaemon[%s/%s] could not apply template, when create deployment: %v", yad.GetNamespace(),
 			yad.GetName(), err)
 		return err

pkg/yurtmanager/controller/yurtappdaemon/workloadcontroller/deployment_controller_test.go

@@ -162,7 +162,7 @@ func TestApplyTemplate(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			get := dc.applyTemplate(tt.scheme, tt.yad, tt.nodepool, tt.revision, tt.set)
+			get := dc.ApplyTemplate(tt.scheme, tt.yad, tt.nodepool, tt.revision, tt.set)
 			t.Logf("expect: %v, get: %v", tt.expect, get)
 			if !reflect.DeepEqual(get, tt.expect) {
 				t.Fatalf("\t%s\texpect %v, but get %v", failed, tt.expect, get)

pkg/yurtmanager/webhook/deploymentrender/v1alpha1/deploymentrender_default.go

@@ -29,6 +29,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	"github.com/openyurtio/openyurt/pkg/apis/apps/v1alpha1"
+	"github.com/openyurtio/openyurt/pkg/yurtmanager/controller/yurtappdaemon/workloadcontroller"
 	"github.com/openyurtio/openyurt/pkg/yurtmanager/controller/yurtappset/adapter"
 )
 
@@ -57,6 +58,16 @@ func (webhook *DeploymentRenderHandler) Default(ctx context.Context, obj runtime
 	if !contain(deployment.OwnerReferences[0].Kind, resources) {
 		return nil
 	}
+
+	// Get nodepool of deployment
+	np := &v1alpha1.NodePool{}
+	npName := deployment.Labels["apps.openyurt.io/pool-name"]
+	if err := webhook.Client.Get(ctx, client.ObjectKey{
+		Name: npName,
+	}, np); err != nil {
+		return err
+	}
+
 	// Get YurtAppSet/YurtAppDaemon resource of this deployment
 	app := deployment.OwnerReferences[0]
 	var instance client.Object
@@ -74,10 +85,8 @@ func (webhook *DeploymentRenderHandler) Default(ctx context.Context, obj runtime
 	}, instance); err != nil {
 		return err
 	}
-	// Get nodepool of deployment
-	nodepool := deployment.Labels["apps.openyurt.io/pool-name"]
 
-	// resume deployment
+	// restore deployment
 	switch app.Kind {
 	case "YurtAppSet":
 		var replicas int32
@@ -86,21 +95,29 @@ func (webhook *DeploymentRenderHandler) Default(ctx context.Context, obj runtime
 		if yas.Spec.WorkloadTemplate.DeploymentTemplate != nil && yas.Spec.WorkloadTemplate.DeploymentTemplate.Spec.Replicas != nil {
 			replicas = *yas.Spec.WorkloadTemplate.DeploymentTemplate.Spec.Replicas
 		}
-		deploymentAdapter := adapter.DeploymentAdapter{
+		yasDeployController := adapter.DeploymentAdapter{
 			Client: webhook.Client,
 			Scheme: webhook.Scheme,
 		}
 		for _, pool := range yas.Spec.Topology.Pools {
-			if pool.Name == nodepool {
+			if pool.Name == npName {
 				replicas = *pool.Replicas
 			}
 		}
-		if err := deploymentAdapter.ApplyPoolTemplate(yas, nodepool, revision, replicas, deployment); err != nil {
+		if err := yasDeployController.ApplyPoolTemplate(yas, npName, revision, replicas, deployment); err != nil {
 			return err
 		}
 	case "YurtAppDaemon":
 		yad := instance.(*v1alpha1.YurtAppDaemon)
-		yad.Spec.WorkloadTemplate.DeploymentTemplate.Spec.DeepCopyInto(&deployment.Spec)
+		revision := yad.Status.CurrentRevision
+		yadDeployController := workloadcontroller.DeploymentControllor{
+			Client: webhook.Client,
+			Scheme: webhook.Scheme,
+		}
+		if err := yadDeployController.ApplyTemplate(webhook.Scheme, yad, *np, revision, deployment); err != nil {
+			return err
+		}
+
 	}
 
 	// Get YurtAppOverrider resource of app(1 to 1)
@@ -125,16 +142,16 @@ func (webhook *DeploymentRenderHandler) Default(ctx context.Context, obj runtime
 
 	for _, entry := range render.Entries {
 		for _, pool := range entry.Pools {
-			if pool[0] == '-' && pool[1:] == nodepool {
+			if pool[0] == '-' && pool[1:] == npName {
 				continue
 			}
-			if pool == nodepool || pool == "*" {
+			if pool == npName || pool == "*" {
 				// Replace items
 				replaceItems(deployment, entry.Items)
 				// json patch
 				for i, patch := range entry.Patches {
 					if strings.Contains(string(patch.Value.Raw), "{{nodepool}}") {
-						newPatchString := strings.ReplaceAll(string(patch.Value.Raw), "{{nodepool}}", nodepool)
+						newPatchString := strings.ReplaceAll(string(patch.Value.Raw), "{{nodepool}}", npName)
 						entry.Patches[i].Value = apiextensionsv1.JSON{Raw: []byte(newPatchString)}
 					}
 				}

pkg/yurtmanager/webhook/deploymentrender/v1alpha1/deploymentrender_webhook_test.go

@@ -81,6 +81,13 @@ var defaultAppSet = &v1alpha1.YurtAppSet{
 	},
 }
 
+var defaultNodePool = &v1alpha1.NodePool{
+	ObjectMeta: metav1.ObjectMeta{
+		Name: "nodepool-test",
+	},
+	Spec: v1alpha1.NodePoolSpec{},
+}
+
 var defaultAppDaemon = &v1alpha1.YurtAppDaemon{
 	ObjectMeta: metav1.ObjectMeta{
 		Name:      "yurtappdaemon",
@@ -312,7 +319,7 @@ func TestDeploymentRenderHandler_Default(t *testing.T) {
 	for _, tcase := range tcases {
 		t.Run("", func(t *testing.T) {
 			webhook := &DeploymentRenderHandler{
-				Client: fakeclient.NewClientBuilder().WithScheme(scheme).WithObjects(defaultAppSet, daemonDeployment, defaultDeployment, defaultAppDaemon, tcase.overrider).Build(),
+				Client: fakeclient.NewClientBuilder().WithScheme(scheme).WithObjects(defaultAppSet, daemonDeployment, defaultNodePool, defaultDeployment, defaultAppDaemon, tcase.overrider).Build(),
 				Scheme: scheme,
 			}
 			if err := webhook.Default(context.TODO(), defaultDeployment); err != nil {