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.

Sign up for GitHub

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

Jump to bottom

Check configmap/secret struct for updating #497

Merged
merged 3 commits into from
Sep 18, 2020
Merged

Check configmap/secret struct for updating #497

merged 3 commits into from
Sep 18, 2020

Conversation

tomahawk28
Copy link
Contributor

@tomahawk28 tomahawk28 commented Sep 18, 2020 •
edited
Loading

Toward https://github.com/giantswarm/giantswarm/issues/13309

Checklist

  • Update changelog in CHANGELOG.md.

@tomahawk28 tomahawk28 self-assigned this Sep 18, 2020
tomahawk28
tomahawk28 commented Sep 18, 2020
View reviewed changes
service/controller/app/resource/configmap/resource.go
}
}

if !reflect.DeepEqual(source, dest) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comparing two values as a struct, not YAML strings would not trigger the constant updates.

@tomahawk28
Copy link
Contributor Author

Tested in geckon

@tomahawk28 tomahawk28 marked this pull request as ready for review September 18, 2020 09:02
@tomahawk28 tomahawk28 requested a review from a team September 18, 2020 09:02
rossf7
rossf7 approved these changes Sep 18, 2020
View reviewed changes
Copy link
Contributor

@rossf7 rossf7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, good catch. Is there anywhere else we need this? Like in the values service.

LGTM when tested manually and tests are green.

@tomahawk28
Copy link
Contributor Author

Values service requires k8sclient, but we don't need that for map comparison. Let's put those changes into each resource.

@tomahawk28
Copy link
Contributor Author

Tested in geckon, no further changes in configmaps, secrets.

@tomahawk28 tomahawk28 merged commit f1dbec4 into master Sep 18, 2020
@tomahawk28 tomahawk28 deleted the check-configmap-struct branch September 18, 2020 11:24
tomahawk28 added a commit that referenced this pull request Sep 18, 2020
@tomahawk28
Check configmap/secret struct for updating (#497)
673fbb3 
* update go mode

* compare configmap data structure

* fix imports
@tomahawk28 tomahawk28 mentioned this pull request Sep 18, 2020
Merged
1 task
tomahawk28 added a commit that referenced this pull request Sep 21, 2020
@tomahawk28
helm2; Check configmap/secret struct for updating (#497) (#498)
629064f 
* Check configmap/secret struct for updating (#497)

* update go mode

* compare configmap data structure

* fix imports

* add changelog
rossf7 pushed a commit that referenced this pull request Sep 22, 2020
@tomahawk28 @rossf7
Check configmap/secret struct for updating (#497)
58e090c 
* update go mode

* compare configmap data structure

* fix imports
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rossf7 rossf7 rossf7 approved these changes

Assignees

@tomahawk28 tomahawk28

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@tomahawk28 @rossf7