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

Crane kustomization support enhancement #57

Merged
merged 1 commit into from
Jan 10, 2022

Conversation

djzager
Copy link
Member

@djzager djzager commented Dec 23, 2021

No description provided.

Comment on lines +36 to +40
- Should the `crane export`ed manifests be the base **OR** should we construct
a completely stripped base with the original and transform+apply manifests
defined as overlays **OR** (easiest) allow the transform+apply manifests to
be the base?
Copy link
Member Author

Choose a reason for hiding this comment

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

Originally, I thought that this was a question that needed to be answered as part of this enhancement. After spending some time thinking about it I no longer believe that to be the case.

We absolutely could provide this minimal level of Kustomize support, especially with GA considerations, and later expand that support in future releases.

Copy link
Contributor

@shawn-hurley shawn-hurley left a comment

Choose a reason for hiding this comment

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

I think that getting the use case written along with the goal might be helpful to determine what the goal should be or vice versa.

["base"](https://kubectl.docs.kubernetes.io/references/kustomize/glossary/#base)
kustomization without modifying the original (aka "base") manifests.
It's important that Crane help users improve their situation when migrating
their applications. This enhancement proposes a new feature to the `apply`
Copy link
Contributor

Choose a reason for hiding this comment

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

Have we considered a new subcommand rather than an option?

I am wondering if this might be more ergonamic?


### Goals

- Allow user to further modify the results of `crane apply` with `kustomize`
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we may want to reword the goal, or maybe I need to be convinced on this point.

My thought was that this was a feature for "Allow users to easily modernize they're exported and transformed manifests to a repeatable deployment tool".

WDYT? maybe this muddies the water more, but while you can modify results in both places this breaks the problem into two subsets. Changes that need to be made on export and changes that need to be made when repeatedly deploying into clusters?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah. Need to spend some more time refining the goal.


## Proposal

As the resources from `crane export` are iterated over:
Copy link
Contributor

Choose a reason for hiding this comment

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

could this be done on an output of an apply?

Copy link
Contributor

Choose a reason for hiding this comment

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

@shawn-hurley I would think so. transform+apply produces a new yaml tree which is in the same format (with same filenames) as the export dir but with changes applied. Really, anything that takes an export dir as an input could be working on the results of an apply if the prior apply output dir is given as the export-dir param -- of course this includes transform+apply as well. You can absolutely transform and apply a set of resources that have previously been transformed and applied. Actually, I'd think for the kustomize use case the norm would be working on the output of apply rather than export, if I'm understanding the expected use cases correctly. Maybe we should use something like "input-dir" to make it clear that we're not necessarily talking about raw export results here.

Copy link
Contributor

@shawn-hurley shawn-hurley left a comment

Choose a reason for hiding this comment

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

/lgtm

@shawn-hurley shawn-hurley merged commit 6b37e5c into konveyor:master Jan 10, 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

3 participants