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

feat(baremetal): Add kustomization.yaml #5965

Merged
merged 1 commit into from
Aug 6, 2020

Conversation

haslersn
Copy link
Contributor

@haslersn haslersn commented Aug 2, 2020

With this change, one can use kustomize to deploy ingress-nginx on bare-metal clusters. Example:

# kustomization.yml

namespace: ingress-nginx

bases:
  - github.com/kubernetes/ingress-nginx/deploy/static/provider/baremetal?ref=master

Usage: In the same folder run: $ kubectl apply -k .

How Has This Been Tested?

# kustomization.yml

namespace: ingress-nginx

bases:
  - github.com/haslersn/ingress-nginx/deploy/static/provider/baremetal?ref=baremetal/kustomize

Checklist:

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I've read the CONTRIBUTION guide
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@k8s-ci-robot
Copy link
Contributor

Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA.

It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.


Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. label Aug 2, 2020
@k8s-ci-robot
Copy link
Contributor

Welcome @haslersn!

It looks like this is your first PR to kubernetes/ingress-nginx 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes/ingress-nginx has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

@k8s-ci-robot k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Aug 2, 2020
@k8s-ci-robot
Copy link
Contributor

Hi @haslersn. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Aug 2, 2020
@k8s-ci-robot k8s-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Aug 2, 2020
@haslersn haslersn force-pushed the baremetal/kustomize branch 3 times, most recently from c7e2aad to 125928e Compare August 2, 2020 03:37
@aledbf
Copy link
Member

aledbf commented Aug 2, 2020

@haslersn we removed kustomize support due to the slowness of the process.
Please check #4156

@aledbf aledbf added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 2, 2020
@haslersn
Copy link
Contributor Author

haslersn commented Aug 2, 2020

@haslersn we remove kustomize support due to the slowness of the process.
Please check #4156

The fixing PR #4159 removed the recommendation to use kustomize, but one could still voluntarily use it. This possibility was removed much later in #5086. I see no reason not to have the possibility.

@aledbf
Copy link
Member

aledbf commented Aug 2, 2020

This possibility was removed much later in #5086. I see no reason not to have the possibility.

ok, that makes sense.

That said, to accept the PR, please add the kustomization.yaml file to each directory in static/provider adding a comment that the file is provided only with the purpose to support kustomize bases. Something like

# NOTE: kustomize is not supported. This file exists only to be able to reference it from bases
# https://kubectl.docs.kubernetes.io/pages/reference/kustomize.html#bases
# namespace: ingress-nginx
# bases:
#  - github.com/kubernetes/ingress-nginx/deploy/static/provider/baremetal?ref=master

resources:
  - deploy.yaml

Copy link
Contributor

@cmluciano cmluciano left a comment

Choose a reason for hiding this comment

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

/ok-to-test

Tests should kick in after you push, I'll try to test this manually now while we await the additional changes

@k8s-ci-robot k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Aug 3, 2020
@haslersn haslersn force-pushed the baremetal/kustomize branch 4 times, most recently from 3a44373 to fdf7b8f Compare August 5, 2020 22:37
@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Aug 5, 2020
@haslersn
Copy link
Contributor Author

haslersn commented Aug 5, 2020

I force-pushed my changes. However, AWS' deploy-tls-termination.yaml is not supported by the respective kustomization.yaml.

@aledbf aledbf removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 5, 2020
@aledbf
Copy link
Member

aledbf commented Aug 6, 2020

/lgtm
/approve

@aledbf
Copy link
Member

aledbf commented Aug 6, 2020

@haslersn thanks!

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: aledbf, haslersn

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 lgtm "Looks good to me", indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Aug 6, 2020
@k8s-ci-robot k8s-ci-robot merged commit 7772c2a into kubernetes:master Aug 6, 2020
@yene
Copy link

yene commented Aug 19, 2020

Could the non kustomize version still be kept around? The change broke my documentation.

@haslersn
Copy link
Contributor Author

This change did not remove anything. What do you mean by "non kustomize version"?

@haslersn haslersn deleted the baremetal/kustomize branch August 19, 2020 20:55
@haslersn haslersn restored the baremetal/kustomize branch August 19, 2020 20:56
@yene
Copy link

yene commented Aug 20, 2020

nvm sorry, I open a new ticket

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. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants