Implement CheckConfig/DiffConfig #577
Conversation
| @@ -107,12 +107,80 @@ func makeKubeProvider( | |||
|
|
|||
| // CheckConfig validates the configuration for this provider. | |||
| func (k *kubeProvider) CheckConfig(ctx context.Context, req *pulumirpc.CheckRequest) (*pulumirpc.CheckResponse, error) { | |||
| return nil, status.Error(codes.Unimplemented, "CheckConfig is not yet implemented") | |||
| // TODO: Check for failures if needed. | |||
| return &pulumirpc.CheckResponse{Inputs: req.GetNews()}, nil | |||
There was a problem hiding this comment.
I wonder, do we need to start returning "defaults" here? For example, if you are using the default provider and not explicitly setting a cluster or context or kubeconfig in your stack's configuration, I think this is going to return an empty set of inputs. If later, you go and change the kubeconfig by hand and re-run up, we won't see any changes. Is that actually the behavior we want here?
There was a problem hiding this comment.
If we do that, we should be careful about cases where in DiffConfig the "olds" are empty, since when upgrading to a newer CLI and a provider with this change, the initial call to DiffConfig will more or less always pass in an empty olds bag.
There was a problem hiding this comment.
If later, you go and change the kubeconfig by hand and re-run up, we won't see any changes. Is that actually the behavior we want here?
FWIW, that behavior may be consistent with what we will get out of the TF providers by default. Those providers may end up automagically picking up some optional settings by way of the cloud provider SDKs (e.g. I think the AWS Go SDK will pick up certain things that are not reflected in the provider config; @jen20 may be able to confirm).
There was a problem hiding this comment.
(e.g. I think the AWS Go SDK will pick up certain things that are not reflected in the provider config; @jen20 may be able to confirm).
Right, I agree that what we have here is similar to the default behavior for TF based providers, my question was more about the UX we /want/ to have.
There was a problem hiding this comment.
Yes, the config is a combination of explicitly set values and what we pick up from awsbase.Credentials and our own provider preconfigure call back.
| @@ -107,12 +107,80 @@ func makeKubeProvider( | |||
|
|
|||
| // CheckConfig validates the configuration for this provider. | |||
| func (k *kubeProvider) CheckConfig(ctx context.Context, req *pulumirpc.CheckRequest) (*pulumirpc.CheckResponse, error) { | |||
| return nil, status.Error(codes.Unimplemented, "CheckConfig is not yet implemented") | |||
| // TODO: Check for failures if needed. | |||
| return &pulumirpc.CheckResponse{Inputs: req.GetNews()}, nil | |||
There was a problem hiding this comment.
If later, you go and change the kubeconfig by hand and re-run up, we won't see any changes. Is that actually the behavior we want here?
FWIW, that behavior may be consistent with what we will get out of the TF providers by default. Those providers may end up automagically picking up some optional settings by way of the cloud provider SDKs (e.g. I think the AWS Go SDK will pick up certain things that are not reflected in the provider config; @jen20 may be able to confirm).
lblackstone
force-pushed
the
lblackstone/implement-diff-config
branch
2 times, most recently
from
5bc1033
to
af395cc
Compare
This change improves Pulumi's ability to correctly replace dependent resources if the cluster they are deployed to changes. As documented in the DiffConfig comments, this method is not foolproof, but should work in the majority of cases.
lblackstone
force-pushed
the
lblackstone/implement-diff-config
branch
from
bde9ea8
to
e546eb2
Compare
This change improves Pulumi's ability to correctly replace dependent
resources if the cluster they are deployed to changes. As documented
in the DiffConfig comments, this method is not foolproof, but should work
in the majority of cases.
Fixes pulumi/pulumi-eks#107
Note: This change depends on pulumi/pulumi#2793, and won't do anything useful if the CLI does not include that change.