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

new kustomize sample #2136

Merged
merged 9 commits into from
Jun 3, 2021
Merged

new kustomize sample #2136

merged 9 commits into from
Jun 3, 2021

Conversation

mikebz
Copy link
Contributor

@mikebz mikebz commented May 29, 2021

This is a new kustomize example which is more in line with how we want people to think about kustomize rendering with kpt live apply and packages.

It replaces the helloworld-kustomize but was started from scratch. The modifications to the files are just git trying to be smart.

related to #1502

```shell
$ kpt pkg tree kustomize-pkg/

PKG: kustomize-pkg
Copy link
Contributor

Choose a reason for hiding this comment

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

While the output of kpt pkg tree here is accurate, it is kinda misleading. It hides the bases and overlays directories. It seems like we remove any directories that doesn't have any KRM resources inside them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's a good observation and I think we should absolutely see if we can fix the tree output.

Copy link
Contributor

Choose a reason for hiding this comment

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

you can use tree command for now


### kustomize the config

We recommend that you keep the kustomize instructions to rendering only such as adding a namespace, transforming or applying a patch. The second you mix kpt packages and remote bases you will be missing out on a big advantage of having a guaranteed stable base.
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we make this clearer? It is not clear to me how using kustomize remote bases would necessarily be bad here, other than challenges around mixing packages fetched with kpt and remote bases 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.

That gets convoluted though as you're effectively using two package managers concurrently. It's hard even for me to reason about it. Instead, you want to have a clean conceptual separation of concerns. The price is that you cannot just drop any arbitrary existing kustomization resource since it may use remote bases.

+1 to making this intentional limiting of kustomize features more precise. Explicitly state what features of kustomize is not recommended?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added stronger language, but I think enumerating every feature of kustomize is going to create a wackamole game. There are new features added and deprecated as we speak and it's best to have a larger umbrella like hydration and packaging in order to avoid updating the sample language every time something changes in kustomize.


### Apply the package

It's possible to use kustomize build and kpt live apply, but it does require passing the inventory to `kpt live apply` from `kustomize build` output. One of the base Kptfiles needs to be initialized with the inventory object. We will use the nginx base, but in real life exployment it might be better to initialize inventory on your root package.
Copy link
Contributor

Choose a reason for hiding this comment

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

Initializing the inventory information in the base is not the recommended way. It means that every variant created (so dev and prod in this example) would have the same inventory, and thus would collide if installed in the same cluster.
Also, I think we should refer to the information added to the Kptfile about the inventory as inventory information. It is a bit vague and not a great name, but we usually refer to the actual resource in the cluster as the inventory object.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

great feedback, would you recommend having a Kptfile in every overlay?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I moved the kpt inventory to ovrelays. PTAL

apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
resources:
- ../../bases/nginx
Copy link
Contributor

Choose a reason for hiding this comment

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

This structure is probably ok for an example, but I think the recommended way to do this would be to have each env have separate git references. The structure in this example makes it a bit hard to make changes in the base and then do progressive rollouts into different environments.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What would be the layout of a solution like that? Every overlay then gets a separate base? Why create an overlay then? We can just have the final config in every folder. I haven't seen a pattern like that, even though I am not opposed to showing it.


### kustomize the config

We recommend that you keep the kustomize instructions to rendering only such as adding a namespace, transforming or applying a patch. The second you mix kpt packages and remote bases you will be missing out on a big advantage of having a guaranteed stable base.
Copy link
Contributor

Choose a reason for hiding this comment

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

That gets convoluted though as you're effectively using two package managers concurrently. It's hard even for me to reason about it. Instead, you want to have a clean conceptual separation of concerns. The price is that you cannot just drop any arbitrary existing kustomization resource since it may use remote bases.

+1 to making this intentional limiting of kustomize features more precise. Explicitly state what features of kustomize is not recommended?

It's possible to use kustomize build and kpt live apply, but it does require passing the inventory to `kpt live apply` from `kustomize build` output. One of the base Kptfiles needs to be initialized with the inventory object. We will use the nginx base, but in real life exployment it might be better to initialize inventory on your root package.

```shell
$ kpt live init bases/nginx
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see a kpt live apply command below.

Also, is the assumption that the they want to use kpt live apply? Or does the user want to use packaging but not hydration and live? Providing both options makes sense.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Live apply from STDIN currently doesn't work so that's why we don't have the output. What I have seen is that people who are using kustomize and see the value in kpt usually like both areas: packaging and apply. They generally already know what to do with kubectl -k or kustomize build and piping to kubectl -f -. Providing them an example of doing that probably doesn't add any value in addition to what they can already gather from kustomize docs.

@mikebz
Copy link
Contributor Author

mikebz commented Jun 3, 2021

also worth noting that this sample will help with addressing #1545

Get the example package on to local using `kpt pkg get`. Note that this package is for this example, wihin that package we are also using the nginx sub-package which is an alternative to having a remote base.

```shell
$ kpt pkg get https://github.com/GoogleContainerTools/kpt.git/package-examples/kustomize-pkg
Copy link
Contributor

Choose a reason for hiding this comment

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

don't you want to specify the next branch?
Does these commands work currently?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is a copy from the template and the rest of them are using the default branch which will be main soon. I don't think we should single this sample out.

Copy link
Contributor

Choose a reason for hiding this comment

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

are you saying no one should try this example before we make next default?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We have all the rest of the samples that do not point to a branch. I am fine making this point to next just to move forward but until now no one has complained about the other samples.

Two theories:

  1. people looking at samples already understand how to point at the right branches and are in tune with the product lifecycle
  2. people are looking at the samples to just understand the flow and read the pattern.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will change it to next, please update it to point to default branch when you change the branches over.

Copy link
Contributor

@frankfarzan frankfarzan Jun 3, 2021

Choose a reason for hiding this comment

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

Two wrongs don't make a right. People are consuming wordpress and nginx through the book which have correct versioning using tags (there are our main examples exposed to users). Just specify the next branch to make this work in the meantime.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

all the existing samples are updated.


PKG: kustomize-pkg
├── [Kptfile] Kptfile kustomize-pkg
├── PKG: nginx
Copy link
Contributor

Choose a reason for hiding this comment

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

Part of the tree has changed @phanimarupaka

```shell
$ kpt pkg tree kustomize-pkg/

PKG: kustomize-pkg
Copy link
Contributor

Choose a reason for hiding this comment

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

you can use tree command for now

@mikebz mikebz requested a review from frankfarzan June 3, 2021 22:40
@mikebz mikebz merged commit 3c5c652 into kptdev:next Jun 3, 2021
@mikebz mikebz deleted the mb_new_kustomize branch June 3, 2021 22:44
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.

3 participants