Skip to content
This repository has been archived by the owner on Mar 24, 2023. It is now read-only.

Run kustomize with base resources #727

Merged
merged 6 commits into from
Nov 27, 2018
Merged

Conversation

Rob0h
Copy link
Contributor

@Rob0h Rob0h commented Nov 27, 2018

What I Did

  • Run kustomize with base resources

How I Did it

  • Run kustomize with base resources
  • Move splitting of multidoc yaml to kustomize step and also deal with base resources recursively
  • CRDs are left in their original state

How to verify it

  • Tests, Integration
  • Run ship init and check the diff in the UI

Description for the Changelog

  • Run kustomize with base resources

Picture of a Boat (not required but encouraged)

🚢

@Rob0h Rob0h requested a review from laverya November 27, 2018 23:22
app: cockroachdb
Copy link
Member

Choose a reason for hiding this comment

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

I don't really like how we're losing comments here
I suppose there's nothing for it though
Unless it's possible to use Ethan's comment preservation code... But I really highly doubt it

Copy link
Contributor Author

@Rob0h Rob0h Nov 27, 2018

Choose a reason for hiding this comment

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

it might actually - i'll give it a go the yaml contents get replaced by the output from kustomize so you're right it's not possible to change heh

return nil
}

func (l *Kustomizer) replaceOriginal(step api.Kustomize, built []postKustomizeFile) error {
Copy link
Member

Choose a reason for hiding this comment

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

I would love a test for this
It looks like it should work fine with our mocked FS for testing, right?

Copy link
Member

Choose a reason for hiding this comment

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

Which exists already, and I completely missed. Right.

Copy link
Member

@laverya laverya left a comment

Choose a reason for hiding this comment

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

WOW this diff is ugly
The golang code all looks fine though, and none of the integration test diffs looked like anything besides ordering changes

@laverya
Copy link
Member

laverya commented Nov 27, 2018

(Also, none of the integration tests have a rendered.yaml change, so that's good)

@Rob0h Rob0h merged commit bdebc84 into replicatedhq:master Nov 27, 2018
@Rob0h Rob0h deleted the fix/diffs branch November 27, 2018 23:45
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants