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

Sort kubernetes objects before applying. #244

Merged
merged 3 commits into from
Mar 31, 2020
Merged

Conversation

mplzik
Copy link
Contributor

@mplzik mplzik commented Mar 20, 2020

Instead of just applying namespces before everything else, let's sort
all the objects. There's more inter-dependences between various kinds,
e.g. CRDs need to be applied before the actual custom resources,
StorageClasses before the PVCs, ... .

Instead of just applying namespces before everything else, let's sort
all the objects. There's more inter-dependences between various kinds,
e.g. CRDs need to be applied before the actual custom resources,
StorageClasses before the PVCs, ... .

Signed-off-by: Milan Plzik <milan.plzik@grafana.com>
@mplzik mplzik requested a review from sh0rez March 20, 2020 11:11
@mplzik mplzik marked this pull request as ready for review March 20, 2020 11:11
@sh0rez
Copy link
Member

sh0rez commented Mar 20, 2020

This looks good, though I wonder if it would fit better into Reconcile(), because we already sort manifests by name there, and it feels pointless to sort them twice.

It would need to be deterministic for this though:

  • Sort by Kind if possible
  • if kind equal, or kind not in orderlist, sort by name

@sh0rez sh0rez mentioned this pull request Mar 20, 2020
As suggested in
#244 (comment) ,
`Reconcile()` might be a better place for sorting, since we're already
doing sort-of (pun intended) similar thing there.

Signed-off-by: Milan Plzik <milan.plzik@grafana.com>
Copy link
Member

@sh0rez sh0rez left a comment

Choose a reason for hiding this comment

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

Looks good! Some refactoring ;)

pkg/kubernetes/client/apply.go Outdated Show resolved Hide resolved
pkg/kubernetes/client/apply.go Show resolved Hide resolved
pkg/kubernetes/reconcile.go Show resolved Hide resolved
Signed-off-by: Milan Plzik <milan.plzik@grafana.com>
Copy link
Member

@sh0rez sh0rez left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

@sh0rez sh0rez merged commit 40a9e45 into master Mar 31, 2020
@sh0rez sh0rez deleted the mplzik/sort-k8s-objects branch March 31, 2020 10:00
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.

2 participants