pulumi / pulumi-eks

A Pulumi component for easily creating and managing an Amazon EKS Cluster
https://www.pulumi.com/registry/packages/eks/
Apache License 2.0
171 stars 81 forks source link

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

Closed Josh-Tilles closed 5 years ago

Josh-Tilles commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

/cc @lukehoban @pgavlin

lukehoban commented 5 years ago

@metral Note that if we fix https://github.com/pulumi/pulumi-eks/issues/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.