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

v1beta1: Update all controllers to use networking.k8s.io/v1beta1 #4081

Closed
wants to merge 1 commit into from

Conversation

cmluciano
Copy link
Contributor

@cmluciano cmluciano commented May 13, 2019

Signed-off-by: Christopher M. Luciano cmluciano@us.ibm.com

What type of PR is this?
/kind cleanup

What this PR does / why we need it:
Migrates all controllers and code to target the new networking.k8s.io/v1 APIs

Which issue(s) this PR fixes:
Related: kubernetes/enhancements#758

Special notes for your reviewer:

  • I'm hoping that I hit all of the variations, but it's possible that something got missed.
  • I will update all examples in a follow-up PR

Does this PR introduce a user-facing change?:
-->

ingress:  Update in-tree Ingress controllers, and clients to target networking.k8s.io/v1beta1

@k8s-ci-robot k8s-ci-robot added kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels May 13, 2019
@aledbf
Copy link
Member

aledbf commented May 13, 2019

I will update all examples in a follow-up PR

Please wait for that after #4055

@aledbf aledbf self-assigned this May 13, 2019
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: cmluciano
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: aledbf

If they are not already assigned, you can assign the PR to them by writing /assign @aledbf in a comment when ready.

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

@cmluciano cmluciano force-pushed the cml/ingressv1beta1 branch from d7d4143 to 6aeb131 Compare May 13, 2019 19:40
@cmluciano
Copy link
Contributor Author

I'm having trouble with some test cases that might be related to a missed make step. Any ideas on this?

=== RUN   TestStore
=== RUN   TestStore/should_return_an_error_searching_for_non_existing_objects
=== RUN   TestStore/should_return_one_event_for_add,_update_and_delete_of_ingress
=== RUN   TestStore/should_not_receive_updates_for_ingress_with_invalid_class
=== RUN   TestStore/should_not_receive_events_from_secret_not_referenced_from_ingress
=== RUN   TestStore/should_receive_events_from_secret_referenced_from_ingress
=== RUN   TestStore/should_create_an_ingress_with_a_secret_which_does_not_exist
=== RUN   TestStore/should_create_an_ingress_with_a_secret_which_does_not_exist/should_exists_a_secret_in_the_local_store_and_filesystem
E0514 15:18:20.285355     611 file.go:31] Error reading file open /etc/ingress-controller/ssl/store-test-ingress-with-secret.pem: no such file or directory
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
	panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x1994b08]

goroutine 401 [running]:
testing.tRunner.func1(0xc00064b600)
	/usr/local/go/src/testing/testing.go:830 +0x69d
panic(0x1af21c0, 0x2dc6b30)
	/usr/local/go/src/runtime/panic.go:522 +0x1b5
k8s.io/ingress-nginx/internal/ingress/controller/store.TestStore.func6.2(0xc00064b600)
	/go/src/k8s.io/ingress-nginx/internal/ingress/controller/store/store_test.go:729 +0x438
testing.tRunner(0xc00064b600, 0xc00058f090)
	/usr/local/go/src/testing/testing.go:865 +0x164
created by testing.(*T).Run
	/usr/local/go/src/testing/testing.go:916 +0x65b
FAIL	k8s.io/ingress-nginx/internal/ingress/controller/store	31.975s

@aledbf
Copy link
Member

aledbf commented May 15, 2019

@cmluciano I can reproduce this locally. Using k8s/testing_framework to use a real apiserver in the test df35990 and I get decoding errors

=== RUN   TestStore
=== RUN   TestStore/should_return_an_error_searching_for_non_existing_objects
I0515 10:30:42.109355   10919 event.go:258] Event(v1.ObjectReference{Kind:"ConfigMap", Namespace:"store-test-1557930642", Name:"config", UID:"0493a755-771e-11e9-84ec-0028f8260615", APIVersion:"v1", ResourceVersion:"42", FieldPath:""}): type: 'Normal' reason: 'CREATE' ConfigMap store-test-1557930642/config
=== RUN   TestStore/should_return_one_event_for_add,_update_and_delete_of_ingress
I0515 10:30:43.902657   10919 event.go:258] Event(v1.ObjectReference{Kind:"ConfigMap", Namespace:"store-test-1557930643", Name:"config", UID:"054da005-771e-11e9-84ec-0028f8260615", APIVersion:"v1", ResourceVersion:"47", FieldPath:""}): type: 'Normal' reason: 'CREATE' ConfigMap store-test-1557930643/config
=== RUN   TestStore/should_not_receive_updates_for_ingress_with_invalid_class
I0515 10:30:45.702543   10919 event.go:258] Event(v1.ObjectReference{Kind:"ConfigMap", Namespace:"store-test-1557930645", Name:"config", UID:"0656fdf1-771e-11e9-84ec-0028f8260615", APIVersion:"v1", ResourceVersion:"52", FieldPath:""}): type: 'Normal' reason: 'CREATE' ConfigMap store-test-1557930645/config
=== RUN   TestStore/should_not_receive_events_from_secret_not_referenced_from_ingress
I0515 10:30:48.302553   10919 event.go:258] Event(v1.ObjectReference{Kind:"ConfigMap", Namespace:"store-test-1557930647", Name:"config", UID:"07a20437-771e-11e9-84ec-0028f8260615", APIVersion:"v1", ResourceVersion:"57", FieldPath:""}): type: 'Normal' reason: 'CREATE' ConfigMap store-test-1557930647/config
=== RUN   TestStore/should_receive_events_from_secret_referenced_from_ingress
I0515 10:30:50.716687   10919 event.go:258] Event(v1.ObjectReference{Kind:"ConfigMap", Namespace:"store-test-1557930650", Name:"config", UID:"09b2f49d-771e-11e9-84ec-0028f8260615", APIVersion:"v1", ResourceVersion:"64", FieldPath:""}): type: 'Normal' reason: 'CREATE' ConfigMap store-test-1557930650/config
=== RUN   TestStore/should_create_an_ingress_with_a_secret_which_does_not_exist
I0515 10:30:53.102568   10919 event.go:258] Event(v1.ObjectReference{Kind:"ConfigMap", Namespace:"store-test-1557930652", Name:"config", UID:"0abb7d35-771e-11e9-84ec-0028f8260615", APIVersion:"v1", ResourceVersion:"70", FieldPath:""}): type: 'Normal' reason: 'CREATE' ConfigMap store-test-1557930652/config
I0515 10:30:54.658816   10919 streamwatcher.go:114] Unexpected EOF during watch stream event decoding: unexpected EOF
I0515 10:30:54.658907   10919 streamwatcher.go:114] Unexpected EOF during watch stream event decoding: unexpected EOF
I0515 10:30:54.658949   10919 streamwatcher.go:114] Unexpected EOF during watch stream event decoding: unexpected EOF
I0515 10:30:54.658912   10919 streamwatcher.go:114] Unexpected EOF during watch stream event decoding: unexpected EOF
W0515 10:30:54.659000   10919 reflector.go:302] k8s.io/ingress-nginx/internal/ingress/controller/store/store.go:176: watch of *v1beta1.Ingress ended with: very short watch: k8s.io/ingress-nginx/internal/ingress/controller/store/store.go:176: Unexpected watch close - watch lasted less than a second and no items received
I0515 10:30:54.659062   10919 streamwatcher.go:114] Unexpected EOF during watch stream event decoding: unexpected EOF
I0515 10:30:54.659066   10919 streamwatcher.go:114] Unexpected EOF during watch stream event decoding: unexpected EOF
I0515 10:30:54.659157   10919 streamwatcher.go:114] Unexpected EOF during watch stream event decoding: unexpected EOF
I0515 10:30:54.659164   10919 streamwatcher.go:114] Unexpected EOF during watch stream event decoding: unexpected EOF
I0515 10:30:54.659234   10919 streamwatcher.go:114] Unexpected EOF during watch stream event decoding: unexpected EOF
I0515 10:30:54.659244   10919 streamwatcher.go:114] Unexpected EOF during watch stream event decoding: unexpected EOF
I0515 10:30:54.659302   10919 streamwatcher.go:114] Unexpected EOF during watch stream event decoding: unexpected EOF
I0515 10:30:54.658910   10919 streamwatcher.go:114] Unexpected EOF during watch stream event decoding: unexpected EOF
I0515 10:30:54.659449   10919 streamwatcher.go:114] Unexpected EOF during watch stream event decoding: unexpected EOF
I0515 10:30:54.658921   10919 streamwatcher.go:114] Unexpected EOF during watch stream event decoding: unexpected EOF

Maybe this is an issue with networking.k8s.io/v1beta1?

@cmluciano
Copy link
Contributor Author

Thanks @aledbf I'll take a look at the client-go stuff to see if there is a weird inconsistency.

@aledbf
Copy link
Member

aledbf commented May 15, 2019

/hold

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 15, 2019
@aledbf
Copy link
Member

aledbf commented May 15, 2019

@cmluciano let me merge #4088 before trying to get this working. With that we can verify this change will not break in k8s < 1.14

@cmluciano cmluciano force-pushed the cml/ingressv1beta1 branch from 6aeb131 to fc307e3 Compare May 17, 2019 20:04
@aledbf
Copy link
Member

aledbf commented May 18, 2019

/retest

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 22, 2019
@aledbf aledbf mentioned this pull request May 22, 2019
@cmluciano cmluciano force-pushed the cml/ingressv1beta1 branch from fc307e3 to aefa555 Compare May 24, 2019 14:02
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 24, 2019
@cmluciano cmluciano force-pushed the cml/ingressv1beta1 branch from aefa555 to 0394eaf Compare May 24, 2019 15:22
Signed-off-by: Christopher M. Luciano <cmluciano@us.ibm.com>
@cmluciano cmluciano force-pushed the cml/ingressv1beta1 branch from 0394eaf to 9b49f92 Compare May 24, 2019 16:03
@k8s-ci-robot
Copy link
Contributor

k8s-ci-robot commented May 24, 2019

@cmluciano: The following tests failed, say /retest to rerun them all:

Test name Commit Details Rerun command
pull-ingress-nginx-test 9b49f92 link /test pull-ingress-nginx-test
pull-ingress-nginx-e2e-1-13 9b49f92 link /test pull-ingress-nginx-e2e-1-13
pull-ingress-nginx-e2e-1-14 9b49f92 link /test pull-ingress-nginx-e2e-1-14

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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.

@aledbf
Copy link
Member

aledbf commented May 27, 2019

@cmluciano please wait, do not continue changing the PR. We cannot switch everything to networking. That would mean we break old clusters (< 1.14.0).

@k8s-ci-robot
Copy link
Contributor

@cmluciano: PR needs rebase.

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 the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 27, 2019
@cmluciano
Copy link
Contributor Author

Closing in favor of #4127

@cmluciano cmluciano closed this Jun 10, 2019
@cmluciano cmluciano deleted the cml/ingressv1beta1 branch June 10, 2019 15:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants