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

Import for complex existing applications: importing this resource will fail #886

Closed
hermanbanken opened this issue Nov 20, 2019 · 13 comments
Assignees
Labels
area/core kind/bug Some behavior is incorrect or out of spec resolution/fixed This issue was fixed

Comments

@hermanbanken
Copy link

Problem description

We manage a GKE cluster with 26 microservices, which all consist of multiple resources (deployment, service, etc). Recently we decided to go with Pulumi to start codifying the infrastructure & apps. We are having troubles however to import our current clusters into Pulumi.

The import statement correctly detects our existing resources, but will always cause "importing this resource will fail" due to our dynamic annotations:

              - metadata  : {
                  - annotations: {
                      - app.kubernetes.io/branch     : "master"
                      - app.kubernetes.io/deploy-date: "20190829091017"
                      - app.kubernetes.io/deployer   : "herman"
                      - app.kubernetes.io/version    : "1.0.136-gcf28c95"
                    }
                }

Ideally, import would respect the ignoreChanges option, so that importing does not fail.
We also tried to remove the annotations (e.g. so that they are imported in full), but this
does not seem to work, because then the diff algorithm thinks we want the annotations to
be removed and the import fails too. If we do not specify any property (removing spec, metadata,
kind, etc) so that import can import everything also fails.

Errors & Logs

warning: inputs to import do not match the existing resource; importing this resource will fail

Affected product version(s)

-> % pulumi version
v1.5.2
-> % pulumi plugin ls
NAME        KIND      VERSION  SIZE   INSTALLED    LAST USED
gcp         resource  1.4.1    88 MB  2 weeks ago  now
kubernetes  resource  1.2.3    47 MB  2 weeks ago  now

Reproducing the issue

  • Requirements:
    • Existing cluster with resource, with some annotations.
    • Pulumi configuration that deploys similar resource but with different annotations & correct import field.
    • No current state.
  • Action pulumi up

Suggestions for a fix

import should respect the ignoreChanges option of resources.

@pgavlin
Copy link
Member

pgavlin commented Nov 20, 2019

Ideally, import would respect the ignoreChanges option, so that importing does not fail.

Can you copy/paste your ignoreChanges annotation here? import is intended to respect this option.

@pgavlin pgavlin self-assigned this Nov 20, 2019
@hermanbanken
Copy link
Author

hermanbanken commented Nov 20, 2019

I am now dynamically creating it in a transform:

const annotations = {
	"app.kubernetes.io/deploy-date": config.deployDate,
	"app.kubernetes.io/deployer": config.deployer,
	"app.kubernetes.io/branch": app.gitBranch,
	"app.kubernetes.io/version": app.gitRef,
	"kubernetes.io/change-cause": config.changeCause,
};

// [...]

{
  ignoreChanges: (args.opts.ignoreChanges || []).concat(["metadata", ...Object.keys(annotations).map(v => `metadata.annotations["${v}"]`)]),
}

Before that I tried using just ignoreChanges: ["metadata"] but that also did not seem to work.

@pgavlin
Copy link
Member

pgavlin commented Nov 20, 2019

At a glance, it looks like the Kubernetes provider is not respecting the ignoreChanges option when performing its diff.

For context, ignoreChanges requires some cooperation between the Pulumi CLI and its resource providers; it seems as if the k8s provider has not implemented its side of the protocol.

cc @lblackstone

@pgavlin
Copy link
Member

pgavlin commented Nov 20, 2019

Does the approach you're taking now work, @hermanbanken ?

@hermanbanken
Copy link
Author

Nope, it does not. We just tried those different properties for ignoreChanges, but all failed. (I read in some PR that ignoreChanges supports that path syntax).

@hermanbanken
Copy link
Author

Given that we have our existing YAML templates kubectl apply'd a very minimal provider would also work for us: one that would just kubectl apply as well.

@lblackstone
Copy link
Member

@pgavlin According to the note on the original ignoreChanges PR, changes shouldn't be required for providers. Is this inaccurate?

I'm unclear what needs to be done to resolve this issue.

@lukehoban
Copy link
Member

Is this inaccurate?

It was accurate when that PR landed - but this was changed in follow-up work that @pgavlin did to provider diffing. He should be able to point you at details of the required contract.

@hermanbanken
Copy link
Author

FYI: I'm working around this issue with a custom provider for Kubernetes that will just kubectl apply everything and will never delete resources. Deleting is not yet required in our case/can be done manually.

Not being able to construct fresh state files (using imports) is a blocker for us, because we are still very much experimenting with Pulumi and the state files are not guaranteed to be compatible between host computers. That's why we're not sharing the state files between computers, and need to construct fresh state files often.

@farvour
Copy link

farvour commented Oct 26, 2020

Any movement on this? Terraform easily adopts a resource, and pays no attention to this type of discrepancy. I am not sure why Pulumi cannot do the same during an import phase. Perhaps there is something beyond what I'm understanding, but if I adopt a resource into Pulumi, I'm technically "ok" with those changes occuring. With a proper preview, I would know exactly what's going to happen once it's adopted into a fresh state and then subsequently modified to....

Even a flag to pass to apply would be great?

@aylei
Copy link

aylei commented Dec 14, 2020

@lblackstone IIUC, the issue might be that pulumi-kubernetes does not respect ignoreChanges during Diff(). This can be addressed by setting the value of ignoreChanges properties from the live state to Input, I think.

However, I am curious that why the runtime does not process ignoreChanges in the ImportStep before sending the Diff request to the provider. Implementing the ignoreChanges logic in each provider might be repeating ourselves.

@viveklak
Copy link
Contributor

viveklak commented Mar 9, 2021

However, I am curious that why the runtime does not process ignoreChanges in the ImportStep before sending the Diff request to the provider. Implementing the ignoreChanges logic in each provider might be repeating ourselves.

I believe support for this was added in pulumi/pulumi#5976

@lukehoban lukehoban added kind/bug Some behavior is incorrect or out of spec area/core labels Apr 30, 2021
@viveklak viveklak added the resolution/fixed This issue was fixed label Apr 30, 2021
@viveklak
Copy link
Contributor

Fixed in pulumi/pulumi#5976

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/core kind/bug Some behavior is incorrect or out of spec resolution/fixed This issue was fixed
Projects
None yet
Development

No branches or pull requests

7 participants