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

VpcCni state includes absolute path to YAML, leading to unnecessary brittleness and opaque changes #149

Closed
Josh-Tilles opened this issue Jun 6, 2019 · 4 comments · Fixed by #201
Assignees
Labels
kind/bug Some behavior is incorrect or out of spec kind/enhancement Improvements or new features
Milestone

Comments

@Josh-Tilles
Copy link

I can run pulumi preview on one machine and see that there will be no changes, but if I move the project to a different directory and run pulumi preview again, I see that Pulumi wants to update a pulumi-nodejs:dynamic:Resource but it can’t explain any more about that change. By inspecting the output of pulumi preview --json, I can see that the only changes are in the (serialized) content of __provider, and the only changes there are in the paths to the CNI YAML:

--- provider1.js	2019-06-06 16:22:34.771169181 -0400
+++ provider2.js	2019-06-06 16:22:32.715455152 -0400
@@ -80,7 +80,7 @@
 
 function __f3() {
     return (function() {
-        with({ applyVpcCniYaml: __applyVpcCniYaml, yamlPath: "/tmp/demo1/node_modules/@pulumi/eks/cni/aws-k8s-cni.yaml", crypto: require("crypto") }) {
+        with({ applyVpcCniYaml: __applyVpcCniYaml, yamlPath: "/tmp/demo2/node_modules/@pulumi/eks/cni/aws-k8s-cni.yaml", crypto: require("crypto") }) {
 
             return (inputs) => {
                 applyVpcCniYaml(yamlPath, inputs);
@@ -93,7 +93,7 @@
 
 function __f4() {
     return (function() {
-        with({ applyVpcCniYaml: __applyVpcCniYaml, yamlPath: "/tmp/demo1/node_modules/@pulumi/eks/cni/aws-k8s-cni.yaml" }) {
+        with({ applyVpcCniYaml: __applyVpcCniYaml, yamlPath: "/tmp/demo2/node_modules/@pulumi/eks/cni/aws-k8s-cni.yaml" }) {
 
             return (id, state, inputs) => {
                 applyVpcCniYaml(yamlPath, inputs);

Tying the Pulumi state to file paths on a deployment machine does not seem desirable or necessary.

@Josh-Tilles
Copy link
Author

Josh-Tilles commented Jun 6, 2019

Also, note that the explanation from Pulumi is very sparse:

$ pulumi preview --diff
Previewing update (Demo):
  pulumi:pulumi:Stack: (same)
    [urn=urn:pulumi:Demo::eks-cluster::pulumi:pulumi:Stack::eks-cluster-Demo]
        ~ pulumi-nodejs:dynamic:Resource: (update)
            [id=f6e3db540990916c]
            [urn=urn:pulumi:Demo::eks-cluster::eks:index:Cluster$pulumi-nodejs:dynamic:Resource::Demo-cluster-vpc-cni]
            kubeconfig: "{\"apiVersion\":\"v1\",\"clusters\":[{\"cluster\":{\"server\":\"https://20A7E6913A4FA81FF038FAX50C81Q209.yl4.us-east-1.eks.amazonaws.com\",\"certificate-authority-data\":\"LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSUN5RENDQWJDZ0F3SUJBZ0lCQURBTkJna3Foa2lHOXcwQkFRc0ZBREFWTVJNd0VRWURWUVFERXdwcmRXSmwKY201bGRHVnpNQjRYRFRFNU1EVXlNekUyTURnME9Gb1hEVEk1TURVeU1ERTJNRGcwT0Zvd0ZURVRNQkVHQTFVRQpBeE1LYTNWaVpYSnVaWFJsY3pDQ0FTSXdEUVlKS29aSWh2Y05BUUVCQlFBRGdnRVBBRENDQVFvQ2dnRUJBTFIvCkswNGN0VkNlWDg1WUhES0p4Uy9IznRWOE5HbDFtNGNlQVdYTEhxeEV1c2wreFlYNkJXUTJGRmY5TjNGME1DQlkKb2VEbjNaWWUvdFB6NmF2N1hPdTM3TklHZlZHamhrREgwREo5YnRYbUlISVUvdWcyNVV3N1hTbzJhRzcvRm11KwpjZzRyZW10dVdlUS9aSUVRL0t6ZkF0S2FkbFRqMVVraDkxdGt3ZERGYkFrMEpxbGZIaHFQd0sxYzZrVUFDOHYxCnlmeEhwT0RHSW02dXZ4amdSb25vTDVXeTA2RGZOa3BGYjFqNXNKWVF1aDJOM0tVTTZlY3M3OExlOVBRTlR5V1kKeXBNMFdvT0pKOWRDTjJpXWk4OTR5VXk2OENUOFg4S3hSUll5eXFYT20kM1VnS2IxSkRiUktHQzZDMXJmTFdQTgpsN3lFMTlRb0FuVWZoU2d0NldNQ0F3RUFBYU1qTUNFd0RnWURWUjBQQVFIL0JBUURBZ0trTUE4R0ExVWRFd0VCCi93UUZNQU1CQWY4d0RRWUpLb1pJaHZjTkFRRUxCUUFEZ2dFQkFKU1VWRnE4VGVxM09nQTRWOGJqRUNEN0JJbVgKMEMzd3hjY2o0NkpLY0RKcVRuZ2hOdjlLTmNUT2xiaGU1bFV5OXVrSE5UNUFrSkRYZHcyNWNZM2xlRXJaS2tYVwpqYWc0WDBNQW8zdDhpUmlQV2tya1E5YlFZd1RiUDRxQTA5SXlwTFFCa3BTcXhtbTEzNGtFTFcxMC9udW1neUpjCm8vTEovN0g3bmxJS1VQTlBPRW9PcFBrL1VQWEFrVUZybFkvMWRFOG03b2JlMzNzQWxpemZCTncwQVpsb2pzVGUKbXc0MmRORnMvQVQ3Tm1kZ2xHeXRnRVNhaFJZSVB2Wkc5Z1NTbndSZ2VUbFkza1ZHeS9xak5kemZ2WWV1UXlQNApOeng3WHlMU014VjZwVjRob0xUYStnZFhXRUVIRnE4WlB6TGxIRzdaOG4rd0trdzF5TTFrY091S3o3VT0KLS0tLS1FTkQgQ0VSVElGSUNBVEUtLS0tLQo=\"},\"name\":\"kubernetes\"}],\"contexts\":[{\"context\":{\"cluster\":\"kubernetes\",\"user\":\"aws\"},\"name\":\"aws\"}],\"current-context\":\"aws\",\"kind\":\"Config\",\"users\":[{\"name\":\"aws\",\"user\":{\"exec\":{\"apiVersion\":\"client.authentication.k8s.io/v1alpha1\",\"command\":\"aws-iam-authenticator\",\"args\":[\"token\",\"-i\",\"Demo-cluster-eksCluster-4c55680\"]}}}]}"
Resources:             
    ~ 1 to update
    49 unchanged
Permalink: https://app.pulumi.com/…

I wouldn’t be surprised if it’s difficult to be more informative with dynamic resources, though.

@metral
Copy link
Contributor

metral commented Jun 13, 2019

I believe I've run into something similar to this. I'll try to repro on my end.

I wouldn’t be surprised if it’s difficult to be more informative with dynamic resources, though.

Right, there's definitely some aspects to dynamic providers, like the CNI, that make previewing lack info, compared to defined resources.

Update: The use of __dirname to compute the filepath of the aws-k8s-cni.yaml manifest to kubectl apply on cluster bootstrapping

@metral
Copy link
Contributor

metral commented Jun 13, 2019

/cc @lukehoban @pgavlin

@lukehoban
Copy link
Member

@metral Note that if we fix #160 we should be able to get rid of the dynamic provider here entirely - which will also fix this issue. I would be inclined to focus on #160 first if possible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Some behavior is incorrect or out of spec kind/enhancement Improvements or new features
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants