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

Fix DaemonSet name on diff #1951

Merged
merged 3 commits into from
Apr 6, 2022

Conversation

alexrocco
Copy link
Contributor

@alexrocco alexrocco commented Apr 1, 2022

Proposed changes

The DaemonSet name was set with a lower case s (Daemonset), but the kind
in the state is created with upper case (DaemonSet), and this was
preventing changes on the .spec.selector to trigger a replacement.

This issue was also causing the Kubernetes API to fail as this field is
immutable, but pulumi was trying to update it.

Related issues (optional)

Related to #1008

Ps. I'm still testing it, but this was the only place where the DaemonSet
was with lower case in the code.

Update: Tested locally and this fixes the issue.

@github-actions
Copy link

github-actions bot commented Apr 1, 2022

PR is now waiting for a maintainer to run the acceptance tests. This PR will only perform build and linting.

Note for the maintainer: To run the acceptance tests, please comment /run-acceptance-tests on the PR

1 similar comment
@github-actions
Copy link

github-actions bot commented Apr 1, 2022

PR is now waiting for a maintainer to run the acceptance tests. This PR will only perform build and linting.

Note for the maintainer: To run the acceptance tests, please comment /run-acceptance-tests on the PR

@alexrocco
Copy link
Contributor Author

Also, this diff.go could be using the types from https://github.com/pulumi/pulumi-kubernetes/blob/master/provider/pkg/kinds/kinds.go.

If the maintainers are fine with that, I could do this change as well.

@viveklak
Copy link
Contributor

viveklak commented Apr 4, 2022

/run-acceptance-tests

@github-actions
Copy link

github-actions bot commented Apr 4, 2022

@viveklak
Copy link
Contributor

viveklak commented Apr 4, 2022

Thanks for the PR! Do you also have a sample program you can use to demonstrate the fix? I am happy to help you convert that to an integration test.

@github-actions
Copy link

github-actions bot commented Apr 5, 2022

PR is now waiting for a maintainer to run the acceptance tests. This PR will only perform build and linting.

Note for the maintainer: To run the acceptance tests, please comment /run-acceptance-tests on the PR

@alexrocco
Copy link
Contributor Author

Thanks for the PR! Do you also have a sample program you can use to demonstrate the fix? I am happy to help you convert that to an integration test.

Just added the integration tests, but don't know if it was the best way of verifying if the DaemonSet was replaced when changing the .spec.selector.

@alexrocco alexrocco force-pushed the alexrocco/fix-daemonset-diff-name branch from e6642a8 to ca6d5a7 Compare April 5, 2022 14:37
@github-actions
Copy link

github-actions bot commented Apr 5, 2022

PR is now waiting for a maintainer to run the acceptance tests. This PR will only perform build and linting.

Note for the maintainer: To run the acceptance tests, please comment /run-acceptance-tests on the PR

@alexrocco alexrocco force-pushed the alexrocco/fix-daemonset-diff-name branch 2 times, most recently from e59a141 to 7a676ed Compare April 5, 2022 14:38
@github-actions
Copy link

github-actions bot commented Apr 5, 2022

PR is now waiting for a maintainer to run the acceptance tests. This PR will only perform build and linting.

Note for the maintainer: To run the acceptance tests, please comment /run-acceptance-tests on the PR

1 similar comment
@github-actions
Copy link

github-actions bot commented Apr 5, 2022

PR is now waiting for a maintainer to run the acceptance tests. This PR will only perform build and linting.

Note for the maintainer: To run the acceptance tests, please comment /run-acceptance-tests on the PR

@alexrocco alexrocco force-pushed the alexrocco/fix-daemonset-diff-name branch from 7a676ed to 44bfa0e Compare April 5, 2022 14:40
@github-actions
Copy link

github-actions bot commented Apr 5, 2022

PR is now waiting for a maintainer to run the acceptance tests. This PR will only perform build and linting.

Note for the maintainer: To run the acceptance tests, please comment /run-acceptance-tests on the PR

@viveklak viveklak self-requested a review April 5, 2022 19:18
@viveklak
Copy link
Contributor

viveklak commented Apr 5, 2022

Apologies @alexrocco - would you be able to rebase this on top of master? We had some breaks in the linting tool we use which has just been fixed on master. Thanks!

The DaemonSet name was set with a lower case s (Daemonset), but the kind
in the state is created with upper case (DaemonSet), and this was
preventing changes on the .spec.selector to trigger a replace.

This issue was also causing the Kubernetes API to fail as this field is
immutable, but pulumi was trying to update it.
@alexrocco alexrocco force-pushed the alexrocco/fix-daemonset-diff-name branch from 44bfa0e to 52a7121 Compare April 6, 2022 05:57
@github-actions
Copy link

github-actions bot commented Apr 6, 2022

PR is now waiting for a maintainer to run the acceptance tests. This PR will only perform build and linting.

Note for the maintainer: To run the acceptance tests, please comment /run-acceptance-tests on the PR

@viveklak
Copy link
Contributor

viveklak commented Apr 6, 2022

/run-acceptance-tests

@github-actions
Copy link

github-actions bot commented Apr 6, 2022

@viveklak viveklak merged commit e4bb2bd into pulumi:master Apr 6, 2022
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.

None yet

2 participants