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

docs for migration to apiVersion networking.k8s.io v1 #7397

Closed
longwuyuan opened this issue Jul 28, 2021 · 7 comments
Closed

docs for migration to apiVersion networking.k8s.io v1 #7397

longwuyuan opened this issue Jul 28, 2021 · 7 comments
Assignees
Labels
area/docs kind/documentation Categorizes issue or PR as related to documentation. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.

Comments

@longwuyuan
Copy link
Contributor

NGINX Ingress controller version:
all builds once k8s v1.22 is released

Kubernetes version (use kubectl version):
v1.22 and above
Environment:
all supported environs

  • Cloud provider or hardware configuration:
  • OS (e.g. from /etc/os-release):
  • Kernel (e.g. uname -a):
  • Install tools:
  • Others:

What happened:
As per #7341, we need docs for the migration to apiVersion networking.k8s.io/v1

What you expected to happen:
Required docs for the migration to apiVersion networking.k8s.io/v1 becomes available

How to reproduce it:
Currently the docs are not present

Anything else we need to know:
Discussion is in #7341

/area docs

@longwuyuan longwuyuan added the kind/bug Categorizes issue or PR as related to a bug. label Jul 28, 2021
@k8s-ci-robot
Copy link
Contributor

@longwuyuan: This issue is currently awaiting triage.

If Ingress contributors determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

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 needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. needs-priority labels Jul 28, 2021
@longwuyuan
Copy link
Contributor Author

/remove-kind bug

@k8s-ci-robot k8s-ci-robot added needs-kind Indicates a PR lacks a `kind/foo` label and requires one. and removed kind/bug Categorizes issue or PR as related to a bug. labels Jul 28, 2021
@k8s-ci-robot
Copy link
Contributor

@longwuyuan: The label(s) kind/docs cannot be applied, because the repository doesn't have them.

In response to this:

/kind docs

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.

@longwuyuan
Copy link
Contributor Author

/kind documentation

@k8s-ci-robot k8s-ci-robot added kind/documentation Categorizes issue or PR as related to documentation. and removed needs-kind Indicates a PR lacks a `kind/foo` label and requires one. labels Jul 28, 2021
@longwuyuan
Copy link
Contributor Author

/assign
/priority important-soon

@k8s-ci-robot k8s-ci-robot added priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. and removed needs-priority labels Jul 28, 2021
@longwuyuan
Copy link
Contributor Author

Use the advise below ;

TODO:

Fix helm chart tests
Write migration guide (looking for some community help!)
Migration guide is mostly about explaining how ingress class works, and scenarios, like:

If you are migrating from v0.X to v1.0, and you have only one ingress deployment in your environment, you can use the --watch-without-ingress-class flag (explain how to enable it in helm chart and/or deployment)
Still, creating an IngressClass with spec.controller = k8s.io/ingress-nginx and turning it default is a good movement
If you already have the annotation of ingress class, you don't need to change anything but it's a good movement to create the ingressClass and start migrating your objects
Etc etc


In charts/ingress-nginx/values.yaml:

ingressClassResource:

  • enabled: false
  • name: nginx
  • enabled: true
    default: false
    Will answer just this and let the others to later:

The idea of default here being false is because when you set it to true, it will assume every new Ingress resource created in the cluster must have / use that ingress class. This is bad, if you have other controllers running in the cluster.

Imagine the same situation with StorageClass, where you have a default storageclass, and then a Helm chart adds another storageclass saying "hey, this is the new default".

Now, two things may happen: Cluster will refuse to create new ingress resources if you have two default classes (I guess this is the default behavior) or, if you don't have any default class, but a bunch of other controllers, this helm chart will take over.

IMO this should be an opt-in, and we need to document really well in the migration guide. For the majority of users, it's fine and also great to have a default ingressclass if they run only one ingress controller :)

I'm worried about the corner cases


@longwuyuan longwuyuan changed the title docs for migration to apiVersion networking.k8s.io/v1 docs for migration to apiVersion networking.k8s.io v1 Aug 22, 2021
@longwuyuan
Copy link
Contributor Author

PR merged so closing #7524

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/docs kind/documentation Categorizes issue or PR as related to documentation. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants