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.

Sign up for GitHub

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

Jump to bottom

Ingress alternate backends #88775

Merged
merged 1 commit into from
Mar 6, 2020
Merged

Ingress alternate backends #88775

merged 1 commit into from
Mar 6, 2020

Conversation

cmluciano
Copy link

@cmluciano cmluciano commented Mar 3, 2020 •
edited
Loading

Co-authored-by: Christopher M. Luciano cmluciano@us.ibm.com

What type of PR is this?
/kind api-change

What this PR does / why we need it:
This PR implements the Resource field within the Ingress package and also prepares for 1.19 refactoring of the service types

Which issue(s) this PR fixes:
xref:

Special notes for your reviewer:

  • Test Create and Update to be sure field isn't allowed on create for 1.18
  • Add validation tests for resource
  • Generate code properly

Does this PR introduce a user-facing change?:

ingress: Add alternate backends via TypedLocalObjectReference

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

Ingress KEP

@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Mar 3, 2020
@cmluciano
Copy link
Author

cc @robscott

@k8s-ci-robot k8s-ci-robot added sig/apps Categorizes an issue or PR as relevant to SIG Apps. sig/network Categorizes an issue or PR as relevant to SIG Network. and removed needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Mar 3, 2020
@liggitt liggitt requested review from robscott and liggitt and removed request for caseydavenport and brendandburns March 3, 2020 21:13
@liggitt liggitt self-assigned this Mar 3, 2020
@liggitt liggitt added api-review Categorizes an issue or PR as actively needing an API review. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. labels Mar 3, 2020
@k8s-ci-robot k8s-ci-robot removed the needs-priority Indicates a PR lacks a `priority/foo` label and requires one. label Mar 3, 2020
@liggitt liggitt added this to Assigned in API Reviews Mar 3, 2020
@liggitt liggitt added this to the v1.18 milestone Mar 3, 2020
@liggitt
Copy link
Member

liggitt commented Mar 4, 2020

pathType is merged if you want to rebase this on master now

liggitt
liggitt reviewed Mar 4, 2020
View reviewed changes
pkg/apis/networking/types.go Outdated Show resolved Hide resolved
pkg/apis/networking/validation/validation.go Outdated Show resolved Hide resolved
pkg/apis/networking/validation/validation.go Outdated Show resolved Hide resolved
@k8s-ci-robot k8s-ci-robot removed the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Mar 4, 2020
@robscott robscott mentioned this pull request Mar 5, 2020
Merged
@k8s-ci-robot k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 5, 2020
@liggitt
Copy link
Member

liggitt commented Mar 5, 2020

/approve

field addition and validation lgtm. will tag once unit tests are complete

/retest

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 5, 2020
@liggitt liggitt moved this from Assigned to API review completed, 1.18 in API Reviews Mar 5, 2020
@cmluciano cmluciano requested a review from liggitt March 5, 2020 15:57
liggitt
liggitt reviewed Mar 5, 2020
View reviewed changes
pkg/apis/networking/validation/validation_test.go Outdated
"nil spec.Backend.Resource and no paths": {
tweakIngress: func(ing *networking.Ingress) {
ing.Spec.Backend = &networking.IngressBackend{
Resource: &api.TypedLocalObjectReference{},
Copy link
Member

Choose a reason for hiding this comment

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

the description says nil spec.Backend.Resource, but this isn't nil

liggitt
liggitt reviewed Mar 5, 2020
View reviewed changes
pkg/apis/networking/validation/validation_test.go Outdated
},
"nil spec.Backend, one rule with nil HTTP ": {
tweakIngress: func(ing *networking.Ingress) {
ing.Spec.Backend = &networking.IngressBackend{}
Copy link
Member

Choose a reason for hiding this comment

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

should be nil to match description?

@cmluciano cmluciano force-pushed the cml/altbackend branch 2 times, most recently from 8080af5 to 374610e Compare March 5, 2020 20:15
@liggitt
Copy link
Member

liggitt commented Mar 5, 2020

/lgtm
/approve

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

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cmluciano, liggitt

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

ingress: add alternate resource backend
912f05b 
Signed-off-by: Christopher M. Luciano <cmluciano@us.ibm.com>
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 5, 2020
@liggitt
Copy link
Member

liggitt commented Mar 5, 2020

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 5, 2020
@liggitt
Copy link
Member

liggitt commented Mar 5, 2020

/retest

1 similar comment
@cmluciano
Copy link
Author

/retest

@k8s-ci-robot k8s-ci-robot merged commit edf460b into kubernetes:master Mar 6, 2020
@github-actions github-actions bot mentioned this pull request Jun 23, 2020
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@liggitt liggitt liggitt left review comments

@robscott robscott Awaiting requested review from robscott

Assignees

@liggitt liggitt

Labels
api-review Categorizes an issue or PR as actively needing an API review. 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/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API 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. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/apps Categorizes an issue or PR as relevant to SIG Apps. sig/network Categorizes an issue or PR as relevant to SIG Network. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
API Reviews
Status: API review completed, 1.18
Milestone
v1.18
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@cmluciano @liggitt @k8s-ci-robot @robscott