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

Versioned static manifests #8162

Merged

Conversation

afirth
Copy link
Member

@afirth afirth commented Jan 18, 2022

Generate static manifests compatible with supported K8s versions, rather than just 1.20

What this PR does / why we need it:

Currently the helm templates have handling for different kubernetes versions, but the script to generate static manifests uses the helm default version (currently 1.20).
With this change the existing deploy/static manifests remain at 1.20 so we don't break docs, but additional folders are underneath with an explicit version.

For more background see #8000 and #7810. #8099 is related.

Changes

The only real changes here are in hack/generate-deploy-scripts.sh

  1. generate for other supported K8s versions
  2. don't delete the 1.20 folder (a hack I put in to make Static manifest generation uses kustomize instead of python #8099 small enough to review)

everything else is the result of running the script.

Types of changes

  • New feature (non-breaking change which adds functionality)

Which issue/s this PR fixes

/fixes #7810

How Has This Been Tested?

  1. Ran ./generate-deploy-scripts.sh
  2. Verified existing manifests updated
  3. spot-checked diffs to confirm expected changes present e.g.
$ diff deploy/static/provider/aws/1.19/deploy.yaml deploy/static/provider/aws/deploy.yaml
0a1
> #GENERATED FOR K8S 1.20
367a369,371
>   ipFamilies:
>   - IPv4
>   ipFamilyPolicy: SingleStack
369c373,374
<   - name: http
---
>   - appProtocol: http
>     name: http
373c378,379
<   - name: https
---
>   - appProtocol: https
>     name: https
398c404,405
<   - name: https-webhook
---
>   - appProtocol: https
>     name: https-webhook

1.19 does not have appProtocol or ipFamilies 👍

Checklist:

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. needs-kind Indicates a PR lacks a `kind/foo` label and requires one. needs-priority size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jan 18, 2022
@afirth afirth force-pushed the 2022-01-18-versioned-static-manifests branch from 53793be to 6a2a2ce Compare January 18, 2022 15:42
@afirth
Copy link
Member Author

afirth commented Jan 18, 2022

/assign @rikatz
9d12bd3 is the script change
6a2a2ce is the manifest generation

boilerplate test broke because I force pushed in the middle but
/test pull-ingress-nginx-boilerplate
anyway

@afirth
Copy link
Member Author

afirth commented Jan 18, 2022

/kind feature

@k8s-ci-robot k8s-ci-robot added kind/feature Categorizes issue or PR as related to a new feature. and removed needs-kind Indicates a PR lacks a `kind/foo` label and requires one. labels Jan 18, 2022
Copy link
Contributor

@iamNoah1 iamNoah1 left a comment

Choose a reason for hiding this comment

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

/triage accepted
/priority important-soon

@k8s-ci-robot k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. needs-priority labels Feb 2, 2022
@afirth
Copy link
Member Author

afirth commented Feb 2, 2022

I updated the description too but understand this could be daunting to a reviewer.
The only real changes here are in hack/generate-deploy-scripts.sh

a reviewer could (untested)

git checkout main
git checkout 6a2a2ce -- ./hack/generate-deploy-scripts.sh
./hack/generate-deploy-scripts.sh

helm, and kustomize must be available (installable with asdf). RELEASE.md was also updated in #8099 with these instructions

Copy link
Contributor

@rikatz rikatz left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve
Thanks

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 13, 2022
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: afirth, rikatz

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 13, 2022
@k8s-ci-robot k8s-ci-robot merged commit c9f6121 into kubernetes:main Feb 13, 2022
@tao12345666333 tao12345666333 mentioned this pull request Feb 27, 2022
@afirth
Copy link
Member Author

afirth commented Mar 1, 2022

@longwuyuan docs were merged in #8099 https://github.com/kubernetes/ingress-nginx/blob/main/RELEASE.md#e-edit-the-static-manifests to update the python and ruyaml deps to helm+kustomize

@longwuyuan
Copy link
Contributor

Just hoping to at least convert that word "kustomize" into a link to the customise software website.

@afirth
Copy link
Member Author

afirth commented Mar 1, 2022

👍 go for it

rchshld pushed a commit to joomcode/ingress-nginx that referenced this pull request May 19, 2023
* update deploy script to generate static manifests for all supported versions

* generate static manifests for all supported versions
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. kind/feature Categorizes issue or PR as related to a new feature. lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cannot install 1.0.4 on K8s v1.19
5 participants