feature: use patch to update service weight (#53)

Signed-off-by: yyzxw <[email protected]>

main.go

@@ -12,7 +12,7 @@ import (
 )
 
 // handshakeConfigs are used to just do a basic handshake between
-// a plugin and host. If the handshake fails, a user friendly error is shown.
+// a plugin and host. If the handshake fails, a user-friendly error is shown.
 // This prevents users from executing bad plugins or executing a plugin
 // directory. It is a UX feature, not a security feature.
 var handshakeConfig = goPlugin.HandshakeConfig{

pkg/plugin/plugin.go

@@ -9,18 +9,16 @@ import (
 	"github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1"
 	rolloutsPlugin "github.com/argoproj/argo-rollouts/rollout/trafficrouting/plugin/rpc"
 	pluginTypes "github.com/argoproj/argo-rollouts/utils/plugin/types"
+	jsonpatch "github.com/evanphx/json-patch"
 	contourv1 "github.com/projectcontour/contour/apis/projectcontour/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/dynamic"
 
 	"github.com/argoproj-labs/rollouts-plugin-trafficrouter-contour/pkg/utils"
 )
 
-// Type holds this controller type
-const Type = "Contour"
-
 var _ rolloutsPlugin.TrafficRouterPlugin = (*RpcPlugin)(nil)
 
 type RpcPlugin struct {
@@ -70,7 +68,7 @@ func (r *RpcPlugin) SetWeight(rollout *v1alpha1.Rollout, canaryWeightPercent int
 	ctx := context.Background()
 
 	for _, proxy := range ctr.HTTPProxies {
-		slog.Debug("updating httpproxy", slog.String("name", proxy))
+		slog.Debug("updating httpproxy weight", slog.String("name", proxy))
 
 		if err := r.updateHTTPProxy(ctx, proxy, rollout, canaryWeightPercent); err != nil {
 			slog.Error("failed to update httpproxy", slog.String("name", proxy), slog.Any("err", err))
@@ -132,12 +130,12 @@ func (r *RpcPlugin) Type() string {
 func (r *RpcPlugin) getHTTPProxy(ctx context.Context, namespace string, name string) (*contourv1.HTTPProxy, error) {
 	unstr, err := r.dynamicClient.Resource(contourv1.HTTPProxyGVR).Namespace(namespace).Get(ctx, name, metav1.GetOptions{})
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("failed to get the httpproxy: %w", err)
 	}
 
 	var httpProxy contourv1.HTTPProxy
 	if err := runtime.DefaultUnstructuredConverter.FromUnstructured(unstr.UnstructuredContent(), &httpProxy); err != nil {
-		return nil, err
+		return nil, fmt.Errorf("failed to convert the httpproxy: %w", err)
 	}
 	return &httpProxy, nil
 }
@@ -153,22 +151,11 @@ func (r *RpcPlugin) updateHTTPProxy(
 		return err
 	}
 
-	canarySvc, stableSvc, totalWeight, err := getRouteServices(httpProxy, rollout)
-	if err != nil {
-		return err
-	}
-
-	slog.Debug("old weight", slog.Int64("canary", canarySvc.Weight), slog.Int64("stable", stableSvc.Weight))
-
-	canarySvc.Weight, stableSvc.Weight = utils.CalcWeight(totalWeight, float32(canaryWeightPercent))
-
-	slog.Debug("new weight", slog.Int64("canary", canarySvc.Weight), slog.Int64("stable", stableSvc.Weight))
-
-	m, err := runtime.DefaultUnstructuredConverter.ToUnstructured(&httpProxy)
+	patchData, patchType, err := createPatch(httpProxy, rollout, canaryWeightPercent)
 	if err != nil {
-		return err
+		return fmt.Errorf("failed to create patch : %w", err)
 	}
-	updated, err := r.dynamicClient.Resource(contourv1.HTTPProxyGVR).Namespace(rollout.Namespace).Update(ctx, &unstructured.Unstructured{Object: m}, metav1.UpdateOptions{})
+	updated, err := r.dynamicClient.Resource(contourv1.HTTPProxyGVR).Namespace(rollout.Namespace).Patch(ctx, httpProxyName, patchType, patchData, metav1.PatchOptions{})
 	if err != nil {
 		return err
 	}
@@ -184,6 +171,31 @@ func (r *RpcPlugin) updateHTTPProxy(
 	return nil
 }
 
+func createPatch(httpProxy *contourv1.HTTPProxy, rollout *v1alpha1.Rollout, canaryWeightPercent int32) ([]byte, types.PatchType, error) {
+	oldData, err := json.Marshal(httpProxy.DeepCopy())
+	if err != nil {
+		return nil, "", fmt.Errorf("failed to marshal the current configuration: %w", err)
+	}
+
+	canarySvc, stableSvc, totalWeight, err := getRouteServices(httpProxy, rollout)
+	if err != nil {
+		return nil, types.MergePatchType, err
+	}
+	slog.Debug("old weight", slog.Int64("canary", canarySvc.Weight), slog.Int64("stable", stableSvc.Weight))
+
+	canarySvc.Weight, stableSvc.Weight = utils.CalcWeight(totalWeight, float32(canaryWeightPercent))
+	slog.Debug("new weight", slog.Int64("canary", canarySvc.Weight), slog.Int64("stable", stableSvc.Weight))
+
+	newData, err := json.Marshal(httpProxy)
+	if err != nil {
+		return nil, "", fmt.Errorf("failed to marshal the current configuration: %w", err)
+	}
+
+	// now default use json merge patch.
+	patch, err := jsonpatch.CreateMergePatch(oldData, newData)
+	return patch, types.MergePatchType, err
+}
+
 func (r *RpcPlugin) verifyHTTPProxy(
 	ctx context.Context,
 	httpProxyName string,
@@ -261,7 +273,7 @@ func getRouteServices(httpProxy *contourv1.HTTPProxy, rollout *v1alpha1.Rollout)
 
 func getContourTrafficRouting(rollout *v1alpha1.Rollout) (*ContourTrafficRouting, error) {
 	var ctr ContourTrafficRouting
-	if err := json.Unmarshal(rollout.Spec.Strategy.Canary.TrafficRouting.Plugins["argoproj-labs/contour"], &ctr); err != nil {
+	if err := json.Unmarshal(rollout.Spec.Strategy.Canary.TrafficRouting.Plugins[ConfigKey], &ctr); err != nil {
 		return nil, err
 	}
 	return &ctr, nil
@@ -288,7 +300,7 @@ func getServiceMap(httpProxy *contourv1.HTTPProxy) map[string]*contourv1.Service
 
 func validateRolloutParameters(rollout *v1alpha1.Rollout) error {
 	if rollout == nil || rollout.Spec.Strategy.Canary == nil || rollout.Spec.Strategy.Canary.StableService == "" || rollout.Spec.Strategy.Canary.CanaryService == "" {
-		return fmt.Errorf("illegal parameter(s)")
+		return fmt.Errorf("illegal parameter(s),both canary service and stable service must be specified")
 	}
 	return nil
 }

pkg/plugin/plugin_test.go

@@ -5,6 +5,7 @@ import (
 	"encoding/json"
 	"log/slog"
 	"os"
+	"reflect"
 	"testing"
 	"time"
 
@@ -18,6 +19,7 @@ import (
 	contourv1 "github.com/projectcontour/contour/apis/projectcontour/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
+	k8stypes "k8s.io/apimachinery/pkg/types"
 	fakeDynClient "k8s.io/client-go/dynamic/fake"
 )
 
@@ -212,3 +214,78 @@ func newRollout(stableSvc, canarySvc, httpProxyName string) *v1alpha1.Rollout {
 		},
 	}
 }
+
+func Test_createPatch(t *testing.T) {
+	type args struct {
+		httpProxy     *contourv1.HTTPProxy
+		rollout       *v1alpha1.Rollout
+		desiredWeight int32
+	}
+	tests := []struct {
+		name          string
+		args          args
+		want          []byte
+		wantPatchType k8stypes.PatchType
+		wantErr       bool
+	}{
+		{
+			name: "test create http proxy patch",
+			args: args{
+				httpProxy: &contourv1.HTTPProxy{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: mocks.HTTPProxyName,
+					},
+					Spec: contourv1.HTTPProxySpec{
+						Routes: []contourv1.Route{
+							{
+								Services: []contourv1.Service{
+									{
+										Name:   mocks.StableServiceName,
+										Weight: 70,
+									},
+									{
+										Name:   mocks.CanaryServiceName,
+										Weight: 20,
+									},
+									{
+										Name:   "others-service",
+										Weight: 10,
+									},
+								},
+							},
+						},
+					},
+				},
+				rollout: &v1alpha1.Rollout{
+					Spec: v1alpha1.RolloutSpec{
+						Strategy: v1alpha1.RolloutStrategy{
+							Canary: &v1alpha1.CanaryStrategy{
+								StableService: mocks.StableServiceName,
+								CanaryService: mocks.CanaryServiceName,
+							},
+						},
+					},
+				},
+				desiredWeight: 50,
+			},
+			want:          []byte(`{"spec":{"routes":[{"services":[{"name":"argo-rollouts-stable","port":0,"weight":45},{"name":"argo-rollouts-canary","port":0,"weight":45},{"name":"others-service","port":0,"weight":10}]}]}}`),
+			wantPatchType: k8stypes.MergePatchType,
+			wantErr:       false,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, gotPatchType, err := createPatch(tt.args.httpProxy, tt.args.rollout, tt.args.desiredWeight)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("createPatch() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+			if !reflect.DeepEqual(gotPatchType, tt.wantPatchType) {
+				t.Errorf("createPatch() gotPatchType = %v, want %v", gotPatchType, tt.wantPatchType)
+			}
+			if !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("createPatch() got = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}

pkg/plugin/types.go

@@ -0,0 +1,8 @@
+package plugin
+
+// Type holds this plugin type
+const Type = "Contour"
+
+// ConfigKey used to identify the plugin in argo-rollouts configmap.
+// see https://argoproj.github.io/argo-rollouts/features/traffic-management/plugins/
+const ConfigKey = "argoproj-labs/contour"