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

fix(kubernetes): properly handle diff namespaces #237

Merged
merged 7 commits into from
Mar 17, 2020
Merged

fix(kubernetes): properly handle diff namespaces #237

merged 7 commits into from
Mar 17, 2020

Conversation

sh0rez
Copy link
Member

@sh0rez sh0rez commented Mar 16, 2020 •
edited
Loading

Supersedes #224 with a approach that I consider more proper:

  1. Cluster-wide resources are always checked with the running cluster
  2. If a namespace is missing but will be created in the same run, report is as created
  3. Everything else is checked with the running cluster.

This is probably a good way because:

  • Fully absent namespaces are still noticed at apply
  • The common case of namespace missing, but soon existing is covered

In the future, this can be easily extended to handle CRD's as well, because the list of known resources is already queried from the cluster (thanks @mplzik for the idea).

@sh0rez
feat(kubernetes): client api-resources
040dc4e 
Adds a method to the client to query all known api-resources from the server
@sh0rez
feat(kubernetes): separate diffable resources
5083fb5
@sh0rez
feat(kubernetes): use separated resources
cc206ba 
Previous commit added support for separating resources in groups. This
commit uses these new groups to provide a better diff experience.

Especially a soon-created namespace won't cause an error anymore.
@sh0rez
refactor: remove old test file
575c819 
The tested code is superseded by other tested code, we don't need that
anymore
@sh0rez sh0rez requested review from rfratto and mplzik March 16, 2020 17:37
@sh0rez sh0rez added component/kubernetes Working with Kubernetes clusters kind/bug Something isn't working labels Mar 16, 2020
@sh0rez
test(kubernetes): UnmarshalTable
c4b235b 
Adds a unit test to ensure UnmarshalTable behaves properly. Also
modified the behaviour a bit for correctness.
@sh0rez
doc(kubernetes): client.Resources
ec58c8c
@sh0rez
style: make golint happy
3695387
rfratto
rfratto approved these changes Mar 16, 2020
View reviewed changes
Copy link
Member

@rfratto rfratto left a comment

Choose a reason for hiding this comment

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

LGTM although you might want to wait for a review from someone who is more familiar with this

@malcolmholmes malcolmholmes self-requested a review March 17, 2020 13:51
@sh0rez sh0rez merged commit 3371845 into master Mar 17, 2020
@sh0rez sh0rez deleted the apiresources branch March 17, 2020 16:27
This was referenced Mar 17, 2020
Closed
Closed
mplzik
mplzik reviewed Mar 17, 2020
View reviewed changes
Copy link
Contributor

@mplzik mplzik left a comment

Choose a reason for hiding this comment

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

Nice PR; apologies for late review. :)

@sh0rez sh0rez mentioned this pull request Mar 20, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mplzik mplzik mplzik left review comments

@rfratto rfratto rfratto approved these changes

@malcolmholmes malcolmholmes malcolmholmes approved these changes

Assignees
No one assigned
Labels
component/kubernetes Working with Kubernetes clusters kind/bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@sh0rez @rfratto @mplzik @malcolmholmes