From 11c2eecfc63ab10b41bcc0daa5c678768aeeb035 Mon Sep 17 00:00:00 2001 From: Levi Blackstone Date: Tue, 22 Jun 2021 12:21:28 -0600 Subject: [PATCH 1/5] Handle different namespaces for server-side diff The server-side diff logic previously tried to use the same k8s client for calculating the patch. In the case where the namespace of a resource changed, this diff would fail because the client was for the old namespace. --- provider/pkg/provider/provider.go | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/provider/pkg/provider/provider.go b/provider/pkg/provider/provider.go index 2c12ca9ab9..2512df0937 100644 --- a/provider/pkg/provider/provider.go +++ b/provider/pkg/provider/provider.go @@ -2201,14 +2201,22 @@ func (k *kubeProvider) serverSidePatch( var newObject *unstructured.Unstructured _, err = client.Get(context.TODO(), newInputs.GetName(), metav1.GetOptions{}) switch { - case err == nil: - newObject, err = client.Patch(context.TODO(), newInputs.GetName(), patchType, patch, metav1.PatchOptions{ + case errors.IsNotFound(err): + newObject, err = client.Create(context.TODO(), newInputs, metav1.CreateOptions{ DryRun: []string{metav1.DryRunAll}, }) - case errors.IsNotFound(err): + case newInputs.GetNamespace() != oldInputs.GetNamespace(): + client, err := k.clientSet.ResourceClient(newInputs.GroupVersionKind(), newInputs.GetNamespace()) + if err != nil { + return nil, nil, err + } newObject, err = client.Create(context.TODO(), newInputs, metav1.CreateOptions{ DryRun: []string{metav1.DryRunAll}, }) + case err == nil: + newObject, err = client.Patch(context.TODO(), newInputs.GetName(), patchType, patch, metav1.PatchOptions{ + DryRun: []string{metav1.DryRunAll}, + }) default: return nil, nil, err } From 16282f1836501ae7e50ee4d334dc26b7ce4dd125 Mon Sep 17 00:00:00 2001 From: Levi Blackstone Date: Tue, 22 Jun 2021 12:24:13 -0600 Subject: [PATCH 2/5] changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 22b8f3ca59..6c169f38bc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ - Update pulumi dependencies v3.5.1 (https://github.com/pulumi/pulumi-kubernetes/pull/1623) - Skip cluster connectivity check in yamlRenderMode (https://github.com/pulumi/pulumi-kubernetes/pull/1629) +- Handle different namespaces for server-side diff (https://github.com/pulumi/pulumi-kubernetes/pull/1631) ## 3.4.0 (June 17, 2021) From 31317642784f55400496bc4c19cec3f6eaa4b962 Mon Sep 17 00:00:00 2001 From: Levi Blackstone Date: Tue, 22 Jun 2021 14:55:13 -0600 Subject: [PATCH 3/5] Test --- tests/sdk/nodejs/dry-run/step1/index.ts | 17 +++++--- tests/sdk/nodejs/dry-run/step1/package.json | 2 +- tests/sdk/nodejs/dry-run/step2/index.ts | 46 +++++++++++++++++++++ tests/sdk/nodejs/nodejs_test.go | 6 +++ 4 files changed, 64 insertions(+), 7 deletions(-) create mode 100644 tests/sdk/nodejs/dry-run/step2/index.ts diff --git a/tests/sdk/nodejs/dry-run/step1/index.ts b/tests/sdk/nodejs/dry-run/step1/index.ts index 1c47a20f3c..8de077da95 100644 --- a/tests/sdk/nodejs/dry-run/step1/index.ts +++ b/tests/sdk/nodejs/dry-run/step1/index.ts @@ -1,4 +1,4 @@ -// Copyright 2016-2019, Pulumi Corporation. +// Copyright 2016-2021, Pulumi Corporation. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -13,13 +13,18 @@ // limitations under the License. import * as k8s from "@pulumi/kubernetes"; +import * as random from "@pulumi/random"; +import * as pulumi from "@pulumi/pulumi"; -const appLabels = { app: "nginx" }; +// TODO(levi): Use auto-naming for namespace once https://github.com/pulumi/pulumi-kubernetes/issues/1632 is fixed. +const suffix = new random.RandomString("suffix", {length: 7, special: false, upper: false}).result; +const ns = new k8s.core.v1.Namespace("test", {metadata: {name: pulumi.interpolate`test-${suffix}`}}); +const provider = new k8s.Provider("k8s", {enableDryRun: true, namespace: ns.metadata.name}); +const appLabels = { app: "nginx" }; const deployment = new k8s.apps.v1.Deployment("nginx", { spec: { selector: { matchLabels: appLabels }, - replicas: 1, template: { metadata: { labels: appLabels }, spec: { @@ -28,14 +33,14 @@ const deployment = new k8s.apps.v1.Deployment("nginx", { image: "nginx", resources: { limits: { - // This value will be normalized by the API server to "2Gi" in the returned state. Without + // This value will be normalized by the API server to "1Gi" in the returned state. Without // dry-run support, Pulumi will detect a spurious diff due to the normalization. - memory: "2048Mi", + memory: "1024Mi", }, }, }], } } } -}); +}, { provider }); export const name = deployment.metadata.name; diff --git a/tests/sdk/nodejs/dry-run/step1/package.json b/tests/sdk/nodejs/dry-run/step1/package.json index 59969e2a6c..214735e1a2 100644 --- a/tests/sdk/nodejs/dry-run/step1/package.json +++ b/tests/sdk/nodejs/dry-run/step1/package.json @@ -1,5 +1,5 @@ { - "name": "steps", + "name": "dry-run", "version": "0.1.0", "dependencies": { "@pulumi/pulumi": "latest", diff --git a/tests/sdk/nodejs/dry-run/step2/index.ts b/tests/sdk/nodejs/dry-run/step2/index.ts new file mode 100644 index 0000000000..0320f2238b --- /dev/null +++ b/tests/sdk/nodejs/dry-run/step2/index.ts @@ -0,0 +1,46 @@ +// Copyright 2016-2021, Pulumi Corporation. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import * as k8s from "@pulumi/kubernetes"; +import * as random from "@pulumi/random"; +import * as pulumi from "@pulumi/pulumi"; + +// TODO(levi): Use auto-naming for namespace once https://github.com/pulumi/pulumi-kubernetes/issues/1632 is fixed. +const suffix = new random.RandomString("suffix", {length: 7, special: false, upper: false}).result; +const ns = new k8s.core.v1.Namespace("test", {metadata: {name: pulumi.interpolate`test-${suffix}`}}); +const provider = new k8s.Provider("k8s", {enableDryRun: true}); // Use the default namespace. + +const appLabels = { app: "nginx" }; +const deployment = new k8s.apps.v1.Deployment("nginx", { + spec: { + selector: { matchLabels: appLabels }, + template: { + metadata: { labels: appLabels }, + spec: { + containers: [{ + name: "nginx", + image: "nginx", + resources: { + limits: { + // This value will be normalized by the API server to "1Gi" in the returned state. Without + // dry-run support, Pulumi will detect a spurious diff due to the normalization. + memory: "1024Mi", + }, + }, + }], + } + } + } +}, { provider }); +export const name = deployment.metadata.name; diff --git a/tests/sdk/nodejs/nodejs_test.go b/tests/sdk/nodejs/nodejs_test.go index 5d258a022d..93fd074c98 100644 --- a/tests/sdk/nodejs/nodejs_test.go +++ b/tests/sdk/nodejs/nodejs_test.go @@ -451,6 +451,12 @@ func TestDeploymentRollout(t *testing.T) { func TestDryRun(t *testing.T) { test := baseOptions.With(integration.ProgramTestOptions{ Dir: filepath.Join("dry-run", "step1"), + EditDirs: []integration.EditDir{ + { + Dir: filepath.Join("dry-run", "step2"), + Additive: true, + }, + }, }) integration.ProgramTest(t, &test) } From 9098a7a31de8d6df46922b339ef7f7840966deab Mon Sep 17 00:00:00 2001 From: Levi Blackstone Date: Tue, 22 Jun 2021 14:58:01 -0600 Subject: [PATCH 4/5] lint --- provider/pkg/provider/provider.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/provider/pkg/provider/provider.go b/provider/pkg/provider/provider.go index 2512df0937..f6c1d39101 100644 --- a/provider/pkg/provider/provider.go +++ b/provider/pkg/provider/provider.go @@ -2213,6 +2213,9 @@ func (k *kubeProvider) serverSidePatch( newObject, err = client.Create(context.TODO(), newInputs, metav1.CreateOptions{ DryRun: []string{metav1.DryRunAll}, }) + if err != nil { + return nil, nil, err + } case err == nil: newObject, err = client.Patch(context.TODO(), newInputs.GetName(), patchType, patch, metav1.PatchOptions{ DryRun: []string{metav1.DryRunAll}, From 1927a13e29610c6add2df97e3f707915f567cd11 Mon Sep 17 00:00:00 2001 From: Levi Blackstone Date: Wed, 23 Jun 2021 11:36:17 -0600 Subject: [PATCH 5/5] Add test comments --- tests/sdk/nodejs/dry-run/step1/index.ts | 3 +++ tests/sdk/nodejs/dry-run/step2/index.ts | 3 +++ 2 files changed, 6 insertions(+) diff --git a/tests/sdk/nodejs/dry-run/step1/index.ts b/tests/sdk/nodejs/dry-run/step1/index.ts index 8de077da95..6596ade399 100644 --- a/tests/sdk/nodejs/dry-run/step1/index.ts +++ b/tests/sdk/nodejs/dry-run/step1/index.ts @@ -16,6 +16,9 @@ import * as k8s from "@pulumi/kubernetes"; import * as random from "@pulumi/random"; import * as pulumi from "@pulumi/pulumi"; +// This test creates a Provider with `enableDryRun` (server side apply) enabled. A namespace is also specified, which +// causes the Deployment to be created in that namespace. + // TODO(levi): Use auto-naming for namespace once https://github.com/pulumi/pulumi-kubernetes/issues/1632 is fixed. const suffix = new random.RandomString("suffix", {length: 7, special: false, upper: false}).result; const ns = new k8s.core.v1.Namespace("test", {metadata: {name: pulumi.interpolate`test-${suffix}`}}); diff --git a/tests/sdk/nodejs/dry-run/step2/index.ts b/tests/sdk/nodejs/dry-run/step2/index.ts index 0320f2238b..30ff2e4b10 100644 --- a/tests/sdk/nodejs/dry-run/step2/index.ts +++ b/tests/sdk/nodejs/dry-run/step2/index.ts @@ -16,6 +16,9 @@ import * as k8s from "@pulumi/kubernetes"; import * as random from "@pulumi/random"; import * as pulumi from "@pulumi/pulumi"; +// This test creates a Provider with `enableDryRun` (server side apply) enabled. The namespace option is removed, which +// causes the Deployment to be recreated in the default namespace. + // TODO(levi): Use auto-naming for namespace once https://github.com/pulumi/pulumi-kubernetes/issues/1632 is fixed. const suffix = new random.RandomString("suffix", {length: 7, special: false, upper: false}).result; const ns = new k8s.core.v1.Namespace("test", {metadata: {name: pulumi.interpolate`test-${suffix}`}});