Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DaemonSetPatch taking ownership of fields unexpectedly upon an update operation with a parent set. #2718

Closed
zbuchheit opened this issue Dec 14, 2023 · 1 comment · Fixed by #2719
Assignees
Labels
customer/feedback Feedback from customers customer/lighthouse Lighthouse customer bugs and enhancements kind/bug Some behavior is incorrect or out of spec p1 A bug severe enough to be the next item assigned to an engineer resolution/fixed This issue was fixed

Comments

@zbuchheit
Copy link

zbuchheit commented Dec 14, 2023

What happened?

When attempting to update an DaemonSetPatch resource while a parent component is set, the DaemonSetPatch took ownership of all field previously owned by kubectl-client-side-apply. This leads to a broken state that requires manual intervention to repair.

Example

Notes

If you create the resource, then change a field like the image to trigger an update, the behavior will occur.

Code Example

export class MyComponent extends pulumi.ComponentResource {
    daemonsetPatch: DaemonSetPatch
    constructor(name: string, opts?: pulumi.ComponentResourceOptions) {
        super("my:component:Resource", name, opts);
        this.daemonsetPatch = new kubernetes.apps.v1.DaemonSetPatch("k8s-daemonset-patch-child", {
            metadata: {
                name: "kube-proxy",
                namespace: "kube-system",
                annotations: {
                    "pulumi.com/patchForce": "true",
                    "pulumi.com/patchFieldManager": "pulumi-daemonset-patch-1",
                },
            },
            spec: {
                template: {
                    spec: {
                        containers: [{
                            name: "kube-proxy",
                            image: "602401143452.dkr.ecr.us-west-2.amazonaws.com/eks/kube-proxy:v1.24.7-minimal-eksbuild.2",
                            command: [
                                "kube-proxy",
                                "--hostname-override=$(NODE_NAME)",
                                "--v=2",
                                "--config=/var/lib/kube-proxy/config.conf",
                            ],
                            env: [{
                                name: "NODE_NAME",
                                valueFrom: {
                                    fieldRef: {
                                        fieldPath: "spec.nodeName",
                                    },
                                },
                            }],
                        }],
                    },
                },
            },
        }, { provider: kubeProvider, retainOnDelete: true, parent: this })
        // Export the Namespace name
        this.registerOutputs({
        });
    }
}

const daemonSet = new MyComponent("k8s-daemonset-patch");

Output of pulumi about

CLI          
Version      3.92.0
Go Version   go1.21.3
Go Compiler  gc

Plugins
NAME        VERSION
aws         6.13.3
awsx        2.3.0
docker      4.5.0
docker      3.6.1
eks         2.0.0
kubernetes  4.6.0
nodejs      unknown

Host     
OS       darwin
Version  14.0
Arch     arm64

This project is written in nodejs: executable='/Users/zbuchheit/.nvm/versions/node/v18.17.1/bin/node' version='v18.17.1'

Current Stack: zbuchheit-pulumi-corp/daemonset-ts/dev

TYPE                               URN
pulumi:pulumi:Stack                urn:pulumi:dev::daemonset-ts::pulumi:pulumi:Stack::daemonset-ts-dev
my:component:Resource              urn:pulumi:dev::daemonset-ts::my:component:Resource::k8s-daemonset-patch
pulumi:providers:pulumi            urn:pulumi:dev::daemonset-ts::pulumi:providers:pulumi::default
pulumi:pulumi:StackReference       urn:pulumi:dev::daemonset-ts::pulumi:pulumi:StackReference::zbuchheit-pulumi-corp/aws-ts-eks/dev
pulumi:providers:kubernetes        urn:pulumi:dev::daemonset-ts::pulumi:providers:kubernetes::eks-kubernetes
kubernetes:apps/v1:DaemonSetPatch  urn:pulumi:dev::daemonset-ts::my:component:Resource$kubernetes:apps/v1:DaemonSetPatch::k8s-daemonset-patch-child


Found no pending operations associated with dev

Backend        
Name           pulumi.com
URL            https://app.pulumi.com/zbuchheit-pulumi-corp
User           zbuchheit-pulumi-corp
Organizations  zbuchheit-pulumi-corp
Token type     personal

Dependencies:
NAME                VERSION
typescript          4.9.5
@pulumi/awsx        2.3.0
@pulumi/eks         2.0.0
@pulumi/kubernetes  4.6.0
@pulumi/pulumi      3.97.0
@types/node         18.19.3

Pulumi locates its logs in /var/folders/lh/l71cdh810xb33t0jc7qmt5_80000gn/T/ by default

Additional context

I will attach the resulting config and manager files once I get a fresh example stood up.

Related to #2692

Contributing

Vote on this issue by adding a 👍 reaction.
To contribute a fix for this issue, leave a comment (and link to your pull request, if you've opened one already).

@zbuchheit zbuchheit added customer/feedback Feedback from customers kind/bug Some behavior is incorrect or out of spec needs-triage Needs attention from the triage team customer/lighthouse Lighthouse customer bugs and enhancements p1 A bug severe enough to be the next item assigned to an engineer labels Dec 14, 2023
@rquitales rquitales self-assigned this Dec 14, 2023
@rquitales rquitales removed the needs-triage Needs attention from the triage team label Dec 14, 2023
@rquitales
Copy link
Contributor

The issue lies with how we determine if a resource is a Patch resource by its URN:

if kinds.PatchQualifiedTypes.Has(c.URN.QualifiedType().String()) {

For a normal DaemonSet patch resource, the urn is kubernetes:apps/v1:DaemonSetPatch. However, for a Patch resource in a component resource like the one provided in the repro code, the URN is my:component:Resource$kubernetes:apps/v1:DaemonSetPatch. The lookup is done via an exact lookup, hence it fails the guard clause, and continues to take over ownership of kubectl fields.

We'll need to rework this lookup to be more relaxed.

rquitales added a commit that referenced this issue Dec 14, 2023
### Proposed changes

This pull request uses the URN`Type` method, rather than `QualifiedType`
to search if resources are a patch or list resource. Qualified types
include the parent resource if there is one, so it would have failed the
lookup.

Additionally, comprehensive unit tests have been included to validate
this change.

### Related issues (optional)
Fixes: #2718 (manual validation was done to ensure this fix solves this
CUJ)
@pulumi-bot pulumi-bot added the resolution/fixed This issue was fixed label Dec 14, 2023
rquitales added a commit that referenced this issue Dec 15, 2023
This pull request uses the URN`Type` method, rather than `QualifiedType`
to search if resources are a patch or list resource. Qualified types
include the parent resource if there is one, so it would have failed the
lookup.

Additionally, comprehensive unit tests have been included to validate
this change.

Fixes: #2718 (manual validation was done to ensure this fix solves this
CUJ)
rquitales added a commit that referenced this issue Dec 15, 2023
This pull request uses the URN`Type` method, rather than `QualifiedType`
to search if resources are a patch or list resource. Qualified types
include the parent resource if there is one, so it would have failed the
lookup.

Additionally, comprehensive unit tests have been included to validate
this change.

Fixes: #2718 (manual validation was done to ensure this fix solves this
CUJ)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
customer/feedback Feedback from customers customer/lighthouse Lighthouse customer bugs and enhancements kind/bug Some behavior is incorrect or out of spec p1 A bug severe enough to be the next item assigned to an engineer resolution/fixed This issue was fixed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants