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

Move issues and prs over from contrib/ingress #3

Closed
bprashanth opened this issue Nov 10, 2016 · 17 comments
Closed

Move issues and prs over from contrib/ingress #3

bprashanth opened this issue Nov 10, 2016 · 17 comments
Assignees
Labels
help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines.

Comments

@bprashanth
Copy link
Contributor

As the title suggests. See tools like https://github.com/google/github-issue-mover

@bprashanth bprashanth added the help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. label Nov 10, 2016
@porridge
Copy link
Member

Issues sound straightforward (once we have a list of all that apply).
However PRs refer to commits, which are against a different directory hierarchy... @bprashanth do you think we should try to rewrite them?

@bprashanth
Copy link
Contributor Author

I think just issues are fine actually. By "move prs" I meant ping the original pr saying we aren't accepting ingress code in contrib anymore and close it with a pointer to this repo. Before we do that we should probably write a doc on how we expect people to integrate with the generic controller, because some prs in contrib are submitting a custom ingress controller, which right now should just become a backed implementing the interface.

@porridge
Copy link
Member

OK, you can assign this one to me, then.

@aledbf
Copy link
Member

aledbf commented Nov 29, 2016

@porridge this is the list of issues to move from contrib:

717
799
847
885
1327
1463
1525
1535
1559
1590
1592
1639
1657
1718
1760
1770
1790
1795
1799
1805
1807
1827
1870
1880
1884
1934
1944
1947
1949
1963
1966
1971
1977
1991
1997
2005
2010
2012
2014
2027
2040
2044
2057
2068
2072
2073
2098
2135
2139
2143
2144
2148
2152
2175
2204
2215
2221
2222
2258
2263
2278
2284
2293
2297
2312
2313
2323
2329
2331

@aledbf
Copy link
Member

aledbf commented Nov 29, 2016

Before we do that we should probably write a doc on how we expect people to integrate with the generic controller, because some prs in contrib are submitting a custom ingress controller, which right now should just become a backed implementing the interface.

before that :) we need to publish the examples and the configuration guide that was removed

Edit: this is the list of PR to close (only related to nginx)

kubernetes-retired/contrib#2368
kubernetes-retired/contrib#2324
kubernetes-retired/contrib#2319
kubernetes-retired/contrib#2204
kubernetes-retired/contrib#2072
kubernetes-retired/contrib#1991
kubernetes-retired/contrib#1943
kubernetes-retired/contrib#1944
kubernetes-retired/contrib#1934
kubernetes-retired/contrib#1880
kubernetes-retired/contrib#1760

@porridge
Copy link
Member

porridge commented Nov 29, 2016 via email

@aledbf
Copy link
Member

aledbf commented Nov 29, 2016

@porridge I think we need to restore the docs first.
After that move the issues (some can be closed but I don't have privileges in contrib) and then the PRs
Until we release a new version of the nginx ingress controller from this repository we cannot remove the code and docs from contrib (after the release is possible to open a PR that removes all the ingress code and just leave a README.md with information about the new this repo)

@bprashanth
Copy link
Contributor Author

We also can't delete the contrib repo because it's linked from older relese documentation on k8s.io, suggest keeping it around for a while with a big notice in the readme linking to this repo.

Re examples, we should start doing this #9 (comment)

@porridge
Copy link
Member

porridge commented Dec 2, 2016

@bprashanth regarding the "older release documentation on k8s.io" do you mean it points to contrib/ingress at the tip of master? Can you show examples? If it points at individual files, I guess we need to keep the source. However if it points at the directory in general, then I guess we could just leave around a README with links to two places:

  1. for old code released in , see
  2. for current development, see

@porridge
Copy link
Member

porridge commented Dec 2, 2016

Doing all of #9 seems like a major undertaking, so I'd like us to agree here on the minimum that needs to be done in order to unblock this issue.

I think that:

  1. make the configuration guide + examples only as good as they were for contrib/ingress, plus
  2. write a doc on how we expect people to integrate with the generic controller (BTW, does this deserve its own issue?)

should be enough.

Rationale: we should not make things worse with the move, but we also cannot expect to improve everything straight away.

@bprashanth WDYT?
@aledbf you mentioned some documentation was deleted, can you point me at the commit which did that, or at least at the URL they were visible at? I'd like to see what the scope and level of detail was and/or salvage what makes sense.

@bprashanth
Copy link
Contributor Author

I was planning to fill in text for all the READMEs added in this pr https://github.com/kubernetes/ingress/pull/33/files

This basically gives us 3 tasks:

  • deleveloper docs: how to write a generic controller and other patterns (ingress controllers that use svc vip, svc DNS, go straight to endpoints; basically stuff that anyone writing a controller should be aware of)
  • admin docs: different deployment styles and tradeoffs (does one use a deployment/daemonset, keepalive vs service vip, cloud vs local etc. Discussion on Documentation: setting up ingress on metal with the right, secure CNI configuration #17, Bare-metal Ingress HA using keepalived #23 might help. This is stuff anyone picking one of the other existing controllers should be aware of)
  • examples: add a folder per example listed in https://github.com/kubernetes/ingress/tree/master/docs/example that contains a <backend_name>.md file showing how to perform the given task via a given backend. A lot of these can be ported over from contrib, but some of them need to be written (eg: pipelining ingress controllers). If a given example doesn't work on say, GCE, the eg stick-sessions/gce.md file should say something to the effect of "you need to pipeline an nginx controller behind the gce lb to achieve this, heres a link to how to do that"

I think a lot of confusion comes from us having a cross platform api, but actually documenting it as distinct implementations, instead of taking a more wholeistic view.

Rationale: we should not make things worse with the move, but we also cannot expect to improve everything straight away.

@porridge I agree with this to some extent, but at the same time we have a chance to slowly improve things as we move and I fear that just lifting and shifting will lead to 0 improvement.

This was referenced Dec 2, 2016
@bprashanth
Copy link
Contributor Author

we have #9 for examples, filed #40 and
#41 for the other 2. Please feel free to comment and take any of them, I'm also going to start chipping away at the examples.

@mrmm
Copy link
Contributor

mrmm commented Dec 9, 2016

Hi @bprashanth @aledbf @porridge, I want to help in this documentation moving from contrib/ TBH it is my first contribution.
I can help moving documentation and improve it with detailed manifests examples

@porridge
Copy link
Member

@mrmm as mentioned in #3 (comment) there are three main areas. Does any one of those feel particularly attractive to you?

@bprashanth
Copy link
Contributor Author

Sorry I meant to comment sooner, I wrote some dev docs on how you can setup a local env, start with those: https://github.com/kubernetes/ingress/tree/master/docs/dev. Report if something doesn't work. Otherwise suggest picking one of the tasks in https://github.com/kubernetes/ingress/tree/master/examples, looking up the corresponding example from contrib/ (https://github.com/kubernetes/contrib/tree/master/ingress), getting it to work and writing the doc. Suggest following the pattern of other examples in main repo for formatting etc (https://github.com/kubernetes/kubernetes/tree/master/examples).

If there's no existing example in contrib, the feature is still possible but just undocumented (eg: nginx controller and sticky sessions), so you can try to figure out the right annotations and write the whole example.

@mrmm
Copy link
Contributor

mrmm commented Dec 16, 2016

@porridge @bprashanth Thanks for your comments, I would like to help with writing detailed examples.

XiShanYongYe-Chang pushed a commit to XiShanYongYe-Chang/ingress-nginx that referenced this issue Feb 25, 2022
Alvaro-Campesino added a commit to Alvaro-Campesino/ingress-nginx-k8s that referenced this issue Aug 5, 2022
YngveMolnes pushed a commit to YngveMolnes/ingress-nginx that referenced this issue May 25, 2023
# This is the 1st commit message:

Add feature flag to disable annotation prefix check in admission controller

# This is the commit message kubernetes#2:

Add log message for when ingress hits annotation check

Indicates that the correct environment variable is set. Does not log on
absence of environment variable.

# This is the commit message kubernetes#3:

Add flag for disabling legacy ingress class annotation prefix check

# This is the commit message kubernetes#4:

Remove negation from if statement on annotation prefix check

# This is the commit message kubernetes#5:

Add logline to indicate annotation prefix check is skipped
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines.
Projects
None yet
Development

No branches or pull requests

5 participants