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

Don't use eq to default a possibly-nil value #1377

Closed
wants to merge 1 commit into from

Conversation

michaelmdresser
Copy link
Contributor

What does this PR change?

kubecostModel.etlCloudAsset is nil by default. Before this change,
running helm template would give the following error:

Error: template: cost-analyzer/templates/cost-analyzer-deployment-template.yaml:533:19: executing "cost-analyzer/templates/cost-analyzer-deployment-template.yaml" at <eq .Values.kubecostModel.etlCloudAsset false>: error calling eq: incompatible types for comparison

After the change, the error is gone.

cc @Sean-Holcomb does this maintain the default behavior you expect for this env var?

Does this PR rely on any other PRs?

N/A

How does this PR impact users? (This is the kind of thing that goes in release notes!)

N/A (I believe the current release doesn't have this issue, I assume this regressed in between the release and now.)

Links to Issues or ZD tickets this PR addresses or fixes

N/A

How was this PR tested?

→ helm template ./cost-analyzer | grep -A 1 -i ETL_CLOUD
            - name : ETL_CLOUD_USAGE_ENABLED
              value: "false"
→ helm template ./cost-analyzer --set kubecostModel.etlCloudAsset=true | grep -A 1 -i ETL_CLOUD
            - name : ETL_CLOUD_USAGE_ENABLED
              value: "true"

Have you made an update to documentation?

N/A

kubecostModel.etlCloudAsset is nil by default. Before this change,
running helm template would give the following error:

Error: template: cost-analyzer/templates/cost-analyzer-deployment-template.yaml:533:19: executing "cost-analyzer/templates/cost-analyzer-deployment-template.yaml" at <eq .Values.kubecostModel.etlCloudAsset false>: error calling eq: incompatible types for comparison

After the change, the error is gone. Testing of the logic:

→ helm template ./cost-analyzer | grep -A 1 -i ETL_CLOUD
            - name : ETL_CLOUD_USAGE_ENABLED
              value: "false"

→ helm template ./cost-analyzer --set kubecostModel.etlCloudAsset=true | grep -A 1 -i ETL_CLOUD
            - name : ETL_CLOUD_USAGE_ENABLED
              value: "true"
@michaelmdresser
Copy link
Contributor Author

Actually, @Sean-Holcomb should this default to true instead of false? I may have misread the current logic.

@michaelmdresser
Copy link
Contributor Author

Closing. This was fixed originally in v1.92 with #1338 and is being brought back to develop with #1380

@michaelmdresser michaelmdresser deleted the mmd/fix-etlcloudasset-eq-nil branch April 18, 2022 20:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant