Skip to content

Commit

Permalink
Do not patch field managers for Patch resources (#2640)
Browse files Browse the repository at this point in the history
### Proposed changes

This pull request addresses the issue where Patch resources
unintentionally patch field managers during update operations (ie. when
a Patch resource is updated and reused). Such modification results in
the Patch resource taking control of all fields managed owned by a field
manager with the prefix `kubectl`. This, in turn, leads to unintended
unset fields when running `pulumi up`. Patch resources are designed to
contain a specific subset of fields for updating, and they should not
interfere with the field managers of normal resources. It's important to
note that field managers still need to be patched for regular resources,
as they facilitate the transition from Client-Side Apply (CSA) to
Server-Side Apply (SSA) for existing Pulumi-managed resources.

Additionally, this PR includes a new test to validate this behavior. The
test will fail if the logic to avoid field manager patching for Patch
resources is not implemented.

### Related issues:
Fixes: #2639
  • Loading branch information
rquitales authored Oct 26, 2023
1 parent a2f31fd commit 70dce7d
Show file tree
Hide file tree
Showing 9 changed files with 262 additions and 0 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
## Unreleased

- Fix: Do not patch field managers for Patch resources (https://github.com/pulumi/pulumi-kubernetes/pull/2640)

## 4.5.1 (October 24, 2023)
- Revert: Normalize provider inputs and make available as outputs (https://github.com/pulumi/pulumi-kubernetes/pull/2627)

Expand Down
8 changes: 8 additions & 0 deletions provider/pkg/await/await.go
Original file line number Diff line number Diff line change
Expand Up @@ -618,6 +618,14 @@ func makeInterfaceSlice[T any](inputs []T) []interface{} {
// fixCSAFieldManagers patches the field managers for an existing resource that was managed using client-side apply.
// The new server-side apply field manager takes ownership of all these fields to avoid conflicts.
func fixCSAFieldManagers(c *UpdateConfig, liveOldObj *unstructured.Unstructured, client dynamic.ResourceInterface) (*unstructured.Unstructured, error) {
if kinds.PatchQualifiedTypes.Has(c.URN.QualifiedType().String()) {
// When dealing with a patch resource, there's no need to patch the field managers.
// Doing so would inadvertently make us responsible for managing fields that are not relevant to us during updates,
// which occurs when reusing a patch resource. Patch resources do not need to worry about other fields
// not directly defined within a the Patch resource.
return liveOldObj, nil
}

managedFields := liveOldObj.GetManagedFields()
if c.Preview || len(managedFields) == 0 {
return liveOldObj, nil
Expand Down
37 changes: 37 additions & 0 deletions tests/sdk/nodejs/field-manager-patch-resources/deployment.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
apiVersion: apps/v1
kind: Deployment
metadata:
name: test-mgr-nginx
labels:
app: nginx
spec:
progressDeadlineSeconds: 600
replicas: 2
revisionHistoryLimit: 10
selector:
matchLabels:
app: nginx
strategy:
rollingUpdate:
maxSurge: 25%
maxUnavailable: 25%
type: RollingUpdate
template:
metadata:
labels:
app: nginx
spec:
containers:
- image: nginx:1.14.2
imagePullPolicy: IfNotPresent
name: nginx
ports:
- containerPort: 80
protocol: TCP
terminationMessagePath: /dev/termination-log
terminationMessagePolicy: File
dnsPolicy: ClusterFirst
restartPolicy: Always
schedulerName: default-scheduler
securityContext: {}
terminationGracePeriodSeconds: 30
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
name: field-mgr-patch-resources-tests
description: Tests Field Manager with Patch resources
runtime: nodejs
49 changes: 49 additions & 0 deletions tests/sdk/nodejs/field-manager-patch-resources/step1/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
// Copyright 2016-2023, 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 pulumi from "@pulumi/pulumi";

// Create provider with SSA enabled.
const provider = new k8s.Provider("k8s", { enableServerSideApply: true });

const config = new pulumi.Config();
const namespace = config.require("namespace");

const depPatch = new k8s.apps.v1.DeploymentPatch(
"nginx-patch",
{
metadata: {
namespace: namespace,
name: "test-mgr-nginx",
annotations: {
"pulumi.com/patchForce": "true",
},
},
spec: {
template: {
metadata: {
labels: undefined,
},
spec: {
containers: [
{
name: "nginx",
image: "nginx:1.14.1",
},
],
},
}
},
}, { provider: provider, retainOnDelete: true });
11 changes: 11 additions & 0 deletions tests/sdk/nodejs/field-manager-patch-resources/step1/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"name": "server-side-apply",
"version": "0.1.0",
"dependencies": {
"@pulumi/pulumi": "latest",
"@pulumi/random": "latest"
},
"peerDependencies": {
"@pulumi/kubernetes": "latest"
}
}
22 changes: 22 additions & 0 deletions tests/sdk/nodejs/field-manager-patch-resources/step1/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
{
"compilerOptions": {
"outDir": "bin",
"target": "es6",
"module": "commonjs",
"moduleResolution": "node",
"declaration": true,
"sourceMap": true,
"stripInternal": true,
"experimentalDecorators": true,
"pretty": true,
"noFallthroughCasesInSwitch": true,
"noImplicitAny": true,
"noImplicitReturns": true,
"forceConsistentCasingInFileNames": true,
"strictNullChecks": true
},
"files": [
"index.ts"
]
}

49 changes: 49 additions & 0 deletions tests/sdk/nodejs/field-manager-patch-resources/step2/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
// Copyright 2016-2023, 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 pulumi from "@pulumi/pulumi";

// Create provider with SSA enabled.
const provider = new k8s.Provider("k8s", { enableServerSideApply: true });

const config = new pulumi.Config();
const namespace = config.require("namespace");

const depPatch = new k8s.apps.v1.DeploymentPatch(
"nginx-patch",
{
metadata: {
namespace: namespace,
name: "test-mgr-nginx",
annotations: {
"pulumi.com/patchForce": "true",
},
},
spec: {
template: {
metadata: {
labels: undefined,
},
spec: {
containers: [
{
name: "nginx",
image: "nginx:1.14.0",
},
],
},
}
},
}, { provider: provider, retainOnDelete: true });
81 changes: 81 additions & 0 deletions tests/sdk/nodejs/nodejs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,9 @@ import (
"github.com/pulumi/pulumi/sdk/v3/go/common/resource"
"github.com/pulumi/pulumi/sdk/v3/go/common/tokens"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/util/rand"
"k8s.io/client-go/tools/clientcmd"
)

Expand Down Expand Up @@ -2008,3 +2010,82 @@ func TestEmptyItemNormalization(t *testing.T) {

integration.ProgramTest(t, &test)
}

// TestFieldManagerPatchResources ensures field managers are not patched when dealing with a Patch resource
// (e.g., DaemonSetPatch). This precaution prevents the Patch resource from taking ownership of fields managed by kubectl field managers,
// which would otherwise result in the unintended unsetting of all unspecified fields of a resource via Server-Side Apply (SSA).
// For additional context, refer to: https://github.com/pulumi/pulumi-kubernetes/issues/2639.
// Test Steps:
// 1. Deploy an nginx:1.14.2 image with 2 replicas using kubectl which sets the field manager to kubectl-client-side-apply.
// 2. Apply a DeploymentPatch resource to modify the deployment by changing the image to nginx:1.14.1 (Step 1).
// 3. Update the DeploymentPatch resource to further patch the deployment, setting the image to nginx:1.14.0,
// and verify that other fields managed by kubectl-client-side-apply remain unaffected (Step 2).
func TestFieldManagerPatchResources(t *testing.T) {
testFolder := "field-manager-patch-resources"

createDeployment := func() string {
// Create a random namespace to deploy the nginx deployment to.
ns := "test-filed-mgr-" + rand.String(5)
_, err := tests.Kubectl("create namespace", ns)
require.NoError(t, err)
t.Cleanup(func() {
tests.Kubectl("delete namespace", ns)
})

// Create nginx deployment.
_, err = tests.Kubectl("apply -f", filepath.Join(testFolder, "deployment.yaml"), "-n", ns)
require.NoError(t, err)
t.Cleanup(func() {
tests.Kubectl("delete namespace", ns)
})

return ns
}

namespace := createDeployment()

test := baseOptions.With(integration.ProgramTestOptions{
Dir: filepath.Join(testFolder, "step1"),
ExpectRefreshChanges: true,
OrderedConfig: []integration.ConfigValue{
{
Key: "pulumi:disable-default-providers[0]",
Value: "kubernetes",
Path: true,
},
},
Config: map[string]string{
"namespace": namespace,
},
ExtraRuntimeValidation: func(t *testing.T, stackInfo integration.RuntimeValidationStackInfo) {
// Ensure that the nginx deployment was patched with image nginx:1.14.1.
depImage, err := tests.Kubectl("get deployment -o=jsonpath={.spec.template.spec.containers[0].image} -n", namespace, "test-mgr-nginx")
assert.NoError(t, err)
assert.Equal(t, "nginx:1.14.1", string(depImage))

// Ensure that the nginx deployment replicas is still 2.
depReplicas, err := tests.Kubectl("get deployment -o=jsonpath={.spec.replicas} -n", namespace, "test-mgr-nginx")
assert.NoError(t, err)
assert.Equal(t, "2", string(depReplicas))
},
EditDirs: []integration.EditDir{
{
Dir: filepath.Join(testFolder, "step2"),
Additive: true,
ExtraRuntimeValidation: func(t *testing.T, stackInfo integration.RuntimeValidationStackInfo) {
// Ensure that the nginx deployment was patched with image nginx:1.14.1.
depImage, err := tests.Kubectl("get deployment -o=jsonpath={.spec.template.spec.containers[0].image} -n", namespace, "test-mgr-nginx")
assert.NoError(t, err)
assert.Equal(t, "nginx:1.14.0", string(depImage))

// Ensure that the nginx deployment replicas is still 2, and was not unset to the default 1 due to field manager being patched.
depReplicas, err := tests.Kubectl("get deployment -o=jsonpath={.spec.replicas} -n", namespace, "test-mgr-nginx")
assert.NoError(t, err)
assert.Equal(t, "2", string(depReplicas))
},
},
},
})

integration.ProgramTest(t, &test)
}

0 comments on commit 70dce7d

Please sign in to comment.