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

update kustomize scaffolding to work with kustomize 2.0 #614

Merged
merged 1 commit into from
Mar 7, 2019

Conversation

Liujingfang1
Copy link
Contributor

Fix #595

@Liujingfang1 Liujingfang1 requested review from mengqiy and droot and removed request for mengqiy February 28, 2019 20:55
@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Feb 28, 2019
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Liujingfang1

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 28, 2019
@droot
Copy link
Contributor

droot commented Feb 28, 2019

@Liujingfang1 Will this new structure work with kustomize 1.0.0 as well ?

I am thinking of cases where users have projects with current/new kustomization structure, but they have older kustomize (1.0.0) will it work for both ? because upgrading to kustomize 2.0.0 will force them to migrate to newer structure.

Also, was wondering if we can update the issue with set of kustomize commands to run to migrate the old structure to work with new structure.

@Liujingfang1
Copy link
Contributor Author

@droot Yes, this structure works with kustomize 1.0 as well.

@Liujingfang1
Copy link
Contributor Author

kustomize commands to run to migrate the old structure to work with new structure

There is no commands in Kustomize to migrate the structures. It requires manual work :(

@Liujingfang1
Copy link
Contributor Author

@droot I added some instructions for migrating from the old structure to the new structure in the issue.

Copy link
Contributor

@droot droot left a comment

Choose a reason for hiding this comment

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

Looking good. one small thing to confirm, have left a comment.

- ../rbac/auth_proxy_role.yaml
- ../rbac/auth_proxy_role_binding.yaml
bases:
- ../rbac
Copy link
Contributor

Choose a reason for hiding this comment

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

I have to double check, but rbac manifests get generated everytime make runs, so not sure if delete all yamls in rbac directory.

Copy link
Member

Choose a reason for hiding this comment

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

IIRC it doesn't delete everything, it overwrites the content in ../rbac/rbac_role.yaml. We'd better confirm it.

Copy link
Contributor

Choose a reason for hiding this comment

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

I confirmed, it doesn't delete, so the change is good.

@droot
Copy link
Contributor

droot commented Mar 6, 2019

@Liujingfang1 The change is good, but we do not have an e2e test in travis which can verify this. we generally test kustomize related changes manually. Will you be able to perform a manual test before the merge ?

@Liujingfang1
Copy link
Contributor Author

@droot Sure, I'll test it manually :)

@Liujingfang1
Copy link
Contributor Author

Liujingfang1 commented Mar 7, 2019

I ran following commands and all of them succeeded. @droot

kubebuilder init --domain jingfang
kubebuilder create api --group my --version v1 --kind MyKind --resource=true --controller=true
make install
make run
kubectl apply -f config/samples/.

# deploy the manager to cluster
make docker-build IMG=jiliu/kb
make docker-push IMG=jiliu/kb
make deploy

@droot
Copy link
Contributor

droot commented Mar 7, 2019

Awesome. Thanks for the e2e test.

@droot droot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 7, 2019
@k8s-ci-robot k8s-ci-robot merged commit 49e9983 into kubernetes-sigs:master Mar 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants