Prevent resource patching when applying in dry-run mode

Fixes #615

Signed-off-by: Oren Shomron <shomron@gmail.com>

pkg/cluster/apply.go

@@ -92,7 +92,7 @@ func RunApply(config ApplyConfig, opts ...ApplyOpts) error {
 		objectInfo:            &objectInfo{},
 		ksonnetObjectFactory: func() ksonnetObject {
 			factory := cmdutil.NewFactory(config.ClientConfig.Config)
-			return newDefaultKsonnetObject(factory)
+			return newDefaultKsonnetObject(factory, config.DryRun)
 		},
 		conflictTimeout: 1 * time.Second,
 	}

pkg/cluster/ksonnet_object.go

@@ -34,8 +34,8 @@ type defaultKsonnetObject struct {
 
 var _ ksonnetObject = (*defaultKsonnetObject)(nil)
 
-func newDefaultKsonnetObject(factory cmdutil.Factory) *defaultKsonnetObject {
-	merger := newDefaultObjectMerger(factory)
+func newDefaultKsonnetObject(factory cmdutil.Factory, dryRun bool) *defaultKsonnetObject {
+	merger := newDefaultObjectMerger(factory, dryRun)
 
 	return &defaultKsonnetObject{
 		objectMerger: merger,

pkg/cluster/ksonnet_object_test.go

@@ -53,6 +53,7 @@ func Test_defaultKsonnetObject_MergeFromCluster(t *testing.T) {
 		expected     *unstructured.Unstructured
 		objectMerger *fakeObjectMerger
 		isErr        bool
+		dryRun       bool
 	}{
 		{
 			name: "merge object",
@@ -78,6 +79,15 @@ func Test_defaultKsonnetObject_MergeFromCluster(t *testing.T) {
 			},
 			expected: sampleObj,
 		},
+		{
+			name: "dry run",
+			obj:  sampleObj,
+			objectMerger: &fakeObjectMerger{
+				mergeObj: sampleObj,
+			},
+			expected: sampleObj,
+			dryRun:   true,
+		},
 	}
 
 	for _, tc := range cases {
@@ -87,7 +97,7 @@ func Test_defaultKsonnetObject_MergeFromCluster(t *testing.T) {
 
 			co := Clients{}
 
-			ko := newDefaultKsonnetObject(factory)
+			ko := newDefaultKsonnetObject(factory, tc.dryRun)
 			ko.objectMerger = tc.objectMerger
 
 			merged, err := ko.MergeFromCluster(co, tc.obj)

pkg/cluster/merger.go

@@ -63,14 +63,16 @@ type objectMerger interface {
 // will ensure that important cluster values aren't overwritten.
 type defaultObjectMerger struct {
 	factory cmdutil.Factory
+	dryRun  bool
 }
 
 var _ objectMerger = (*defaultObjectMerger)(nil)
 
 // newDefaultObjectMerger creates an instance of objectMerge.
-func newDefaultObjectMerger(factory cmdutil.Factory) *defaultObjectMerger {
+func newDefaultObjectMerger(factory cmdutil.Factory, dryRun bool) *defaultObjectMerger {
 	p := &defaultObjectMerger{
 		factory: factory,
+		dryRun:  dryRun,
 	}
 
 	return p
@@ -129,6 +131,10 @@ func (p *defaultObjectMerger) Merge(namespace string, obj *unstructured.Unstruct
 		return nil, errors.Wrap(err, "encode modified object")
 	}
 
+	if p.dryRun {
+		return obj, nil
+	}
+
 	helper := resource.NewHelper(info.Client, info.Mapping)
 	patcher := &patcher{
 		encoder:       encoder,
@@ -207,9 +213,15 @@ type patcher struct {
 	gracePeriod int
 
 	openapiSchema openapi.Resources
+
+	dryRun bool
 }
 
 func (p *patcher) patchSimple(obj runtime.Object, modified []byte, source, namespace, name string, errOut io.Writer) ([]byte, runtime.Object, error) {
+	if p.dryRun {
+		return modified, obj, nil
+	}
+
 	// Serialize the current configuration of the object from the server.
 	current, err := runtime.Encode(p.encoder, obj)
 	if err != nil {
@@ -293,6 +305,10 @@ func (p *patcher) patchSimple(obj runtime.Object, modified []byte, source, names
 }
 
 func (p *patcher) patch(current runtime.Object, modified []byte, source, namespace, name string, errOut io.Writer) ([]byte, runtime.Object, error) {
+	if p.dryRun {
+		return modified, current, nil
+	}
+
 	var getErr error
 	patchBytes, patchObject, err := p.patchSimple(current, modified, source, namespace, name, errOut)
 	for i := 1; i <= maxPatchRetry && kerrors.IsConflict(err); i++ {
@@ -312,6 +328,9 @@ func (p *patcher) patch(current runtime.Object, modified []byte, source, namespa
 }
 
 func (p *patcher) deleteAndCreate(original runtime.Object, modified []byte, namespace, name string) ([]byte, runtime.Object, error) {
+	if p.dryRun {
+		return modified, original, nil
+	}
 	err := p.delete(namespace, name)
 	if err != nil {
 		return modified, nil, err
@@ -344,6 +363,9 @@ func (p *patcher) deleteAndCreate(original runtime.Object, modified []byte, name
 }
 
 func (p *patcher) delete(namespace, name string) error {
+	if p.dryRun {
+		return nil
+	}
 	c, err := p.clientFunc(p.mapping)
 	if err != nil {
 		return err

pkg/cluster/merger_test.go

@@ -122,7 +122,7 @@ func Test_merger_merge(t *testing.T) {
 
 	tf.ClientConfigVal = &restclient.Config{}
 
-	om := newDefaultObjectMerger(tf)
+	om := newDefaultObjectMerger(tf, false)
 
 	obj := &unstructured.Unstructured{
 		Object: map[string]interface{}{
@@ -156,6 +156,102 @@ func Test_merger_merge(t *testing.T) {
 	require.True(t, isPatched)
 }
 
+func Test_merger_merge_dryrun(t *testing.T) {
+	tf := cmdtesting.NewTestFactory()
+	defer tf.Cleanup()
+
+	codec := legacyscheme.Codecs.LegacyCodec(scheme.Versions...)
+
+	servicePath := "/namespaces/testing/services/service"
+
+	clusterService := &api.Service{
+		Spec: api.ServiceSpec{
+			Ports: []api.ServicePort{
+				{NodePort: 30000},
+			},
+		},
+	}
+
+	isPatched := false
+
+	tf.UnstructuredClient = &fake.RESTClient{
+		NegotiatedSerializer: unstructuredSerializer,
+		Client: fake.CreateHTTPClient(func(req *http.Request) (*http.Response, error) {
+			switch p, m := req.URL.Path, req.Method; {
+			case p == servicePath && m == "GET":
+				return &http.Response{StatusCode: 200, Header: defaultHeader(), Body: objBody(codec, clusterService)}, nil
+			case p == servicePath && m == "PATCH":
+				defer req.Body.Close()
+				_, err := convertToObject(req.Body)
+				require.NoError(t, err)
+
+				isPatched = true
+
+				return &http.Response{StatusCode: 200, Header: defaultHeader(), Body: objBody(codec, clusterService)}, nil
+			default:
+				t.Fatalf("unexpected request using unstructured client: %#v\n%#v", req.URL, req)
+				return nil, nil
+			}
+		}),
+	}
+
+	tf.Client = &fake.RESTClient{
+		Client: fake.CreateHTTPClient(func(req *http.Request) (*http.Response, error) {
+			switch p, m := req.URL.Path, req.Method; {
+			case p == "/openapi/v2" && m == "GET":
+				schemaPath := filepath.Join("testdata", "swagger.json")
+				f, err := os.Open(schemaPath)
+				require.NoError(t, err)
+
+				return &http.Response{StatusCode: 200, Body: f}, nil
+			default:
+				t.Fatalf("unexpected request using client: %#v\n%#v", req.URL, req)
+				return nil, errors.New("not found")
+			}
+		}),
+	}
+
+	tf.OpenAPISchemaFunc = func() (openapi.Resources, error) {
+		return nil, errors.New("not found")
+	}
+
+	tf.ClientConfigVal = &restclient.Config{}
+
+	om := newDefaultObjectMerger(tf, true)
+
+	obj := &unstructured.Unstructured{
+		Object: map[string]interface{}{
+			"apiVersion": "v1",
+			"kind":       "Service",
+			"metadata": map[string]interface{}{
+				"name": "service",
+				"labels": map[string]interface{}{
+					"foo": "bar",
+				},
+			},
+			"spec": map[string]interface{}{
+				"ports": []interface{}{
+					map[string]interface{}{
+						"protocol":   "TCP",
+						"targetPort": 8080,
+						"port":       80,
+					},
+				},
+				"selector": map[string]interface{}{
+					"app": "MyApp",
+				},
+				"type": "NodePort",
+			},
+		},
+	}
+
+	_, err := om.Merge("testing", obj)
+	require.NoError(t, err)
+
+	// Won't patch because of dry-run
+	require.False(t, isPatched)
+}
+
 type fakeObjectMerger struct {
 	mergeObj *unstructured.Unstructured
 	mergeErr error

pkg/cluster/upsert.go

@@ -107,6 +107,7 @@ func (u *defaultUpserter) updateObject(rc ResourceClient, obj *unstructured.Unst
 	}
 
 	if u.DryRun {
+		log.Info("skipping patch (dry-run)")
 		return obj, nil
 	}