Bug 1794546: Console-1933: IPv6 support (simpler alternative) #375

benjaminapetersen · 2020-01-23T16:22:49Z

Simpler/preferred change over #374

RE doc changes needed

/assign @russellb
/cc @spadgett @jhadvig

Operator config bindNetwork: "tcp"
- Per openshift-kni@31b4d39
Console config handles IPv4 or IPv6 based on network.config.openshift.io
- Per openshift-kni@01865a6
Downloads Deployment handles IPv4 or IPv6
- Per openshift-kni@f395cb1
tests updated
- all unit tests updated to verify the expected output
IPv6 test?
- All current tests should continue to pass on IPv4
- Unclear how to test an IPv6 cluster at this time

manifests/07-downloads-deployment.yaml

spadgett

/approve
/lgtm
/hold

Just adding a hold for feedback from others and for any testing.

/cc @russellb @bparees

spadgett · 2020-01-23T17:05:14Z

manifests/07-downloads-deployment.yaml

+          # IPv6 should handle IPv4 passively so long as it is not bound to a
+          # specific address or set to IPv6_ONLY
+          # https://stackoverflow.com/questions/25817848/python-3-does-http-server-support-ipv6
+          addr = ('::', 8080)


We should confirm that :: doesn't break ipv4. It's not clear to me from the stack overflow thread

if it breaks ipv4 will it not fail our CI jobs?

Yup, we have e2e tests for this so it would fail CI if this is a problem.

I ran it in my cluster, was able to verify downloads still download via the UI. pod is running, etc. I think we are good.

Can we configure nodes with the ipv6.disable=1 kernel command line and see what happens if we blacklist the ipv6 kernel module (via an MC?)

Yeah, this will most likely fail if ipv6_disable. Go-based code doesn't have to worry about that because go has code to deal with that and fall back to 0.0.0.0 if you try to bind to :: but IPv6 is unavailable.

So do we need to hold on this, or make a bug for follow?

You can resolve this by going back to dynamically changing the downloader based on whether the cluster was configured for IPv6 or not.

Ah, this inline python script is not great for any kind of additional complexity. Sounds like an overhaul of the downloads bit.

spadgett · 2020-01-23T17:09:35Z

@benjaminapetersen Do we have any CLI download test coverage in CI?

spadgett

/lgtm

spadgett · 2020-01-23T18:16:42Z

/retest

benjaminapetersen · 2020-01-23T18:20:29Z

@spadgett yes, we have:
https://github.com/openshift/console-operator/blob/master/test/e2e/downloads_test.go#L24
which will loop our CLIDownloads CRD & verify each link responds with a payload.

We could potentially add one in console to also verify the downloads links do respond with files:

https://github.com/openshift/console/blob/master/frontend/integration-tests/tests/crd-extensions.scenario.ts#L15
But thats prob not super necessary.

openshift-ci-robot · 2020-01-23T19:11:04Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: benjaminapetersen, spadgett

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:

~~OWNERS~~ [benjaminapetersen,spadgett]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

benjaminapetersen · 2020-01-23T19:36:12Z

/retest

could not wait for build: the build console-operator failed after 2m8s with reason DockerBuildFailed: Docker build strategy has failed.

benjaminapetersen · 2020-01-23T19:36:40Z

/cherry-pick release-4.3

This should backport cleanly. Bug: https://bugzilla.redhat.com/show_bug.cgi?id=1794548

openshift-cherrypick-robot · 2020-01-23T19:36:41Z

@benjaminapetersen: once the present PR merges, I will cherry-pick it on top of release-4.3 in a new PR and assign it to you.

In response to this:

/cherry-pick release-4.3

This should backport cleanly.

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.

openshift-ci-robot · 2020-01-23T19:41:05Z

@benjaminapetersen: This pull request references Bugzilla bug 1794546, which is invalid:

expected the bug to target the "4.4.0" release, but it targets "---" instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

In response to this:

Bug 1794546: Console-1933: IPv6 support (simpler alternative)

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.

benjaminapetersen · 2020-01-23T19:41:34Z

/bugzilla refresh

openshift-ci-robot · 2020-01-23T19:41:40Z

@benjaminapetersen: This pull request references Bugzilla bug 1794546, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

In response to this:

/bugzilla refresh

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.

benjaminapetersen · 2020-01-23T21:26:12Z

/retest

level=fatal msg="failed to fetch Cluster: failed to generate asset \"Cluster\": failed to create cluster: failed to apply using Terraform"

benjaminapetersen · 2020-01-24T19:59:47Z

/retest

unexpected flake:

     util.go:26: patching Data on the console ConfigMap
    util.go:33: polling for patched Data on the console ConfigMap
    unmanaged_test.go:18: waiting for cleanup to reach settled state...
    console-operator.go:369: waited 10 seconds to reach settled state...
=== RUN   TestEditUnmanagedService
--- FAIL: TestEditUnmanagedService (69.10s)
    unmanaged_test.go:12: waiting for setup to reach settled state...
    unmanaged_test.go:13: changing console operator state to 'Unmanaged'...
    util.go:49: patching Annotation on the console Service
    util.go:56: polling for patched Annotation on the console Service
    unmanaged_test.go:18: waiting for cleanup to reach settled state...
    util.go:95: patching DisplayName on the odo-cli-downloads ConsoleCLIDownloads custom resource
    util.go:102: polling for patched DisplayName on the odo-cli-downloads ConsoleCLIDownloads custom resource
    unmanaged_test.go:18: waiting for cleanup to reach settled state...
    console-operator.go:369: waited 10 seconds to reach settled state...
    console-operator.go:369: waited 30 seconds to reach settled state...
FAIL
FAIL	github.com/openshift/console-operator/test/e2e	918.176s
[ERROR] PID 231: hack/test-e2e.sh:5: `KUBERNETES_CONFIG=${KUBECONFIG} go test -timeout 30m -v ./test/e2e/` exited with status 1.
[INFO] 		Stack Trace: 
[INFO] 		  1: hack/test-e2e.sh:5: `KUBERNETES_CONFIG=${KUBECONFIG} go test -timeout 30m -v ./test/e2e/`
[INFO]   Exiting with code 1.

will keep an eye on that.

benjaminapetersen · 2020-01-25T00:17:34Z

/retest

level=fatal msg="failed to fetch Cluster: failed to generate asset \"Cluster\": failed to create cluster: failed to apply using Terraform"

spadgett · 2020-01-25T00:18:18Z

/hold cancel

openshift-bot · 2020-01-25T02:29:10Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-01-25T04:00:16Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-01-25T04:13:12Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-ci-robot · 2020-01-25T06:51:59Z

@benjaminapetersen: All pull requests linked via external trackers have merged. Bugzilla bug 1794546 has been moved to the MODIFIED state.

In response to this:

Bug 1794546: Console-1933: IPv6 support (simpler alternative)

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.

openshift-cherrypick-robot · 2020-01-25T06:52:14Z

@benjaminapetersen: #375 failed to apply on top of branch "release-4.3":

Applying: Add bindNetwork: tcp to operator config yaml
Applying: Update console config-builder to IPv6 with passive IPv4
Applying: Fix unit tests for IPv6/IPv4
error: Failed to merge in the changes.
Using index info to reconstruct a base tree...
M	pkg/console/subresource/configmap/configmap_test.go
M	pkg/console/subresource/consoleserver/config_builder_test.go
Falling back to patching base and 3-way merge...
Auto-merging pkg/console/subresource/consoleserver/config_builder_test.go
Auto-merging pkg/console/subresource/configmap/configmap_test.go
CONFLICT (content): Merge conflict in pkg/console/subresource/configmap/configmap_test.go
Patch failed at 0004 Fix unit tests for IPv6/IPv4

In response to this:

/cherry-pick release-4.3

This should backport cleanly. Bug: https://bugzilla.redhat.com/show_bug.cgi?id=1794548

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.

russellb · 2020-01-25T13:47:10Z

Have you filed a bug somewhere to follow-up on the issue raised by @eparis, where this would break on a host with IPv6 disabled?

benjaminapetersen · 2020-01-27T17:12:11Z

Created this bug:

https://bugzilla.redhat.com/show_bug.cgi?id=1795325

I am presently uncertain about the severity & will be fishing around for further details.

benjaminapetersen added 3 commits January 23, 2020 11:16

Update Downloads deployment to IPv6 with passive IPv4 support

5119e82

Add bindNetwork: tcp to operator config yaml

51a3988

Update console config-builder to IPv6 with passive IPv4

eee4db7

openshift-ci-robot assigned russellb Jan 23, 2020

openshift-ci-robot requested review from jhadvig and spadgett January 23, 2020 16:22

openshift-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Jan 23, 2020

benjaminapetersen mentioned this pull request Jan 23, 2020

[WIP] Console-1933 IPv6 support #374

Closed

4 tasks

benjaminapetersen commented Jan 23, 2020

View reviewed changes

manifests/07-downloads-deployment.yaml Show resolved Hide resolved

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 23, 2020

spadgett approved these changes Jan 23, 2020

View reviewed changes

openshift-ci-robot requested review from bparees and russellb January 23, 2020 17:08

openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 23, 2020

openshift-ci-robot assigned spadgett Jan 23, 2020

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jan 23, 2020

Fix unit tests for IPv6/IPv4

11998a7

openshift-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed lgtm Indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jan 23, 2020

spadgett approved these changes Jan 23, 2020

View reviewed changes

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jan 23, 2020

benjaminapetersen changed the title ~~Console-1933: IPv6 support (simpler alternative)~~ Bug 1794546: Console-1933: IPv6 support (simpler alternative) Jan 23, 2020

openshift-ci-robot added the bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. label Jan 23, 2020

openshift-ci-robot added the bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. label Jan 23, 2020

openshift-ci-robot removed the bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. label Jan 23, 2020

openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 25, 2020

openshift-merge-robot merged commit 9d973a9 into openshift:master Jan 25, 2020

benjaminapetersen deleted the CONSOLE-1933/IPv6-support-simpler-alternative branch January 27, 2020 16:56

benjaminapetersen mentioned this pull request Jan 28, 2020

Bug 1794548: IPv6 support to console operator & console [openshift-4.3.z] #376

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bug 1794546: Console-1933: IPv6 support (simpler alternative) #375

Bug 1794546: Console-1933: IPv6 support (simpler alternative) #375

benjaminapetersen commented Jan 23, 2020 •

edited

spadgett left a comment

spadgett Jan 23, 2020

bparees Jan 23, 2020

benjaminapetersen Jan 23, 2020

eparis Jan 23, 2020

danwinship Jan 24, 2020

benjaminapetersen Jan 24, 2020

russellb Jan 24, 2020

benjaminapetersen Jan 27, 2020

spadgett commented Jan 23, 2020

spadgett left a comment

spadgett commented Jan 23, 2020

benjaminapetersen commented Jan 23, 2020

openshift-ci-robot commented Jan 23, 2020

benjaminapetersen commented Jan 23, 2020

benjaminapetersen commented Jan 23, 2020 •

edited

openshift-cherrypick-robot commented Jan 23, 2020

openshift-ci-robot commented Jan 23, 2020

benjaminapetersen commented Jan 23, 2020

openshift-ci-robot commented Jan 23, 2020

benjaminapetersen commented Jan 23, 2020

benjaminapetersen commented Jan 24, 2020

benjaminapetersen commented Jan 25, 2020

spadgett commented Jan 25, 2020

openshift-bot commented Jan 25, 2020

openshift-bot commented Jan 25, 2020

openshift-bot commented Jan 25, 2020

openshift-ci-robot commented Jan 25, 2020

openshift-cherrypick-robot commented Jan 25, 2020

russellb commented Jan 25, 2020

benjaminapetersen commented Jan 27, 2020

Bug 1794546: Console-1933: IPv6 support (simpler alternative) #375

Bug 1794546: Console-1933: IPv6 support (simpler alternative) #375

Conversation

benjaminapetersen commented Jan 23, 2020 • edited

spadgett left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

spadgett commented Jan 23, 2020

spadgett left a comment

Choose a reason for hiding this comment

spadgett commented Jan 23, 2020

benjaminapetersen commented Jan 23, 2020

openshift-ci-robot commented Jan 23, 2020

benjaminapetersen commented Jan 23, 2020

benjaminapetersen commented Jan 23, 2020 • edited

openshift-cherrypick-robot commented Jan 23, 2020

openshift-ci-robot commented Jan 23, 2020

benjaminapetersen commented Jan 23, 2020

openshift-ci-robot commented Jan 23, 2020

benjaminapetersen commented Jan 23, 2020

benjaminapetersen commented Jan 24, 2020

benjaminapetersen commented Jan 25, 2020

spadgett commented Jan 25, 2020

openshift-bot commented Jan 25, 2020

openshift-bot commented Jan 25, 2020

openshift-bot commented Jan 25, 2020

openshift-ci-robot commented Jan 25, 2020

openshift-cherrypick-robot commented Jan 25, 2020

russellb commented Jan 25, 2020

benjaminapetersen commented Jan 27, 2020

benjaminapetersen commented Jan 23, 2020 •

edited

benjaminapetersen commented Jan 23, 2020 •

edited