Bug 1824934: Fix console oauthEndpointCAFile setting #403

Miciah · 2020-03-27T21:21:37Z

Configure the console to use the default-ingress-cert configmap if the configmap exists, or else to use the default CA if the configmap is absent. Before this commit, the operator did exactly the reverse.

Fix a comment that falsely stated that the configmap would be absent if the administrator provided a default certificate for every ingress controller.

Follow-up to #361.

pkg/console/operator/sync_v400.go: Invert the value of useDefaultCAFile. Fix a comment.

@jhadvig

Configure the console to use the default-ingress-cert configmap if the configmap exists, or else to use the default CA if the configmap is absent. Before this commit, the operator did exactly the reverse. Fix a comment that falsely stated that the configmap would be absent if the administrator provided a default certificate for every ingress controller. Follow-up to commit 489dfd0. * pkg/console/operator/sync_v400.go: Invert the value of useDefaultCAFile. Fix a comment.

spadgett · 2020-03-30T17:35:07Z

Thanks @Miciah. Seeing a test error that looks related.

 --- FAIL: TestEditManagedRoute (243.82s)
    managed_test.go:13: waiting for setup to reach settled state...
    util.go:72: patching TargetPort on the console Route
    util.go:79: polling for patched TargetPort on the console Route
    managed_test.go:17: 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...
    console-operator.go:369: waited 60 seconds to reach settled state...
    console-operator.go:369: waited 90 seconds to reach settled state...
    console-operator.go:369: waited 120 seconds to reach settled state...
    managed_test.go:17: operator has not reached settled state in 4m0s attempts due to [RouteHealthDegraded] - timed out waiting for the condition

benjaminapetersen

/approve
/lgtm

But looking into the e2e failures. something funny.

openshift-ci-robot · 2020-04-02T02:03:34Z

[APPROVALNOTIFIER] This PR is APPROVED

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

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [benjaminapetersen]

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

openshift-bot · 2020-04-02T02:10:59Z

/retest

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

openshift-bot · 2020-04-02T02:23:59Z

/retest

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

Miciah · 2020-04-02T02:40:10Z

I have been performing some manual testing:

% go test -count 1 -v -tags e2e -run TestEditManagedRoute ./test/e2e

Sometimes the test succeeds in about 30 to 90 seconds, sometimes it hangs for several minutes and eventually fails. When it hangs, I can see the failing status condition:

% oc get consoles.operator/cluster -o yaml
...
  - lastTransitionTime: "2020-04-02T02:33:16Z"
    message: route not yet available, https://console-openshift-console.apps.ci-ln-h70r2jt-d5d6b.origin-ci-int-aws.dev.rhcloud.com/health
      returns '503 Service Unavailable'
    reason: StatusError
    status: "True"
    type: RouteHealthDegraded
...

But curling the route works:

% curl -ksv https://console-openshift-console.apps.ci-ln-h70r2jt-d5d6b.origin-ci-int-aws.dev.rhcloud.com/health
* About to connect() to console-openshift-console.apps.ci-ln-h70r2jt-d5d6b.origin-ci-int-aws.dev.rhcloud.com port 443 (#0)
*   Trying 44.228.49.144...
* Connected to console-openshift-console.apps.ci-ln-h70r2jt-d5d6b.origin-ci-int-aws.dev.rhcloud.com (44.228.49.144) port 443 (#0)
* Initializing NSS with certpath: sql:/etc/pki/nssdb
* skipping SSL peer certificate verification
* SSL connection using TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256
* Server certificate:
*       subject: CN=*.apps.ci-ln-h70r2jt-d5d6b.origin-ci-int-aws.dev.rhcloud.com
*       start date: Apr 02 00:19:22 2020 GMT
*       expire date: Apr 02 00:19:23 2022 GMT
*       common name: *.apps.ci-ln-h70r2jt-d5d6b.origin-ci-int-aws.dev.rhcloud.com
*       issuer: CN=ingress-operator@1585786761
> GET /health HTTP/1.1
> User-Agent: curl/7.29.0
> Host: console-openshift-console.apps.ci-ln-h70r2jt-d5d6b.origin-ci-int-aws.dev.rhcloud.com
> Accept: */*
>
< HTTP/1.1 200 OK
< content-type: application/json
< referrer-policy: strict-origin-when-cross-origin
< x-content-type-options: nosniff
< x-dns-prefetch-control: off
< x-frame-options: DENY
< x-xss-protection: 1; mode=block
< date: Thu, 02 Apr 2020 02:35:48 GMT
< content-length: 15
< set-cookie: 1e2670d92730b515ce3a1bb65da45062=5bc7860bccc251cfb87ae1d8366659b6; path=/; HttpOnly; Secure
< cache-control: private
<
* Connection #0 to host console-openshift-console.apps.ci-ln-h70r2jt-d5d6b.origin-ci-int-aws.dev.rhcloud.com left intact
{"status":"ok"}

Could the status update code be failing to update?

benjaminapetersen · 2020-04-02T03:03:35Z

I've been running the tests locally as well, they appear to be working for me. Very curious.

benjaminapetersen · 2020-04-02T03:25:57Z

It may be that we just need to increase the timeout a bit. I've ran these a number of times and only hit a fail once.

openshift-bot · 2020-04-02T03:29:04Z

/retest

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

Miciah · 2020-04-02T03:50:01Z

I only did a small number of tests on my test cluster. I saw two successes and two failures.

openshift-bot · 2020-04-02T07:38:55Z

/retest

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

openshift-bot · 2020-04-02T08:19:37Z

/retest

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

benjaminapetersen · 2020-04-02T13:20:53Z

We will keep an eye on this, perhaps our timeout needs to increase... but will look to see if there is anything else wrong.

benjaminapetersen · 2020-04-02T13:32:23Z

/cherry-pick release-4.4

I think we need to backport this.

openshift-cherrypick-robot · 2020-04-02T13:32:38Z

@benjaminapetersen: new pull request created: #407

Details

In response to this:

/cherry-pick release-4.4

I think we need to backport this.

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-04-16T17:08:52Z

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

Details

In response to this:

Bug 1824934: Fix console oauthEndpointCAFile setting

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-04-16T17:14:36Z

/bugzilla assign-qa

openshift-ci-robot · 2020-04-16T17:14:40Z

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

Details

In response to this:

/bugzilla assign-qa

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.

Miciah referenced this pull request Mar 27, 2020

Sync default-ingress-cert to the console namespace

489dfd0

openshift-ci-robot requested review from benjaminapetersen and spadgett March 27, 2020 21:21

Miciah mentioned this pull request Mar 27, 2020

Delete logic to publish router-ca configmap openshift/cluster-ingress-operator#377

Merged

Miciah mentioned this pull request Mar 30, 2020

Use "default-ingress-cert", not "router-ca" openshift/installer#3380

Merged

benjaminapetersen approved these changes Apr 2, 2020

View reviewed changes

openshift-ci-robot assigned benjaminapetersen Apr 2, 2020

openshift-ci-robot added lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Apr 2, 2020

openshift-merge-robot merged commit e26c857 into openshift:master Apr 2, 2020

openshift-cherrypick-robot mentioned this pull request Apr 2, 2020

Bug 1824935: [release-4.4] Fix console oauthEndpointCAFile setting #407

Merged

benjaminapetersen changed the title ~~Fix console oauthEndpointCAFile setting~~ Bug 1824934: Fix console oauthEndpointCAFile setting Apr 16, 2020

Bug 1824934: Fix console oauthEndpointCAFile setting #403

Bug 1824934: Fix console oauthEndpointCAFile setting #403

Uh oh!

Conversation

Miciah commented Mar 27, 2020

Uh oh!

spadgett commented Mar 30, 2020

Uh oh!

benjaminapetersen left a comment

Choose a reason for hiding this comment

Uh oh!

openshift-ci-robot commented Apr 2, 2020

Uh oh!

openshift-bot commented Apr 2, 2020

Uh oh!

openshift-bot commented Apr 2, 2020

Uh oh!

Miciah commented Apr 2, 2020

Uh oh!

benjaminapetersen commented Apr 2, 2020

Uh oh!

benjaminapetersen commented Apr 2, 2020

Uh oh!

openshift-bot commented Apr 2, 2020

Uh oh!

Miciah commented Apr 2, 2020

Uh oh!

openshift-bot commented Apr 2, 2020

Uh oh!

openshift-bot commented Apr 2, 2020

Uh oh!

benjaminapetersen commented Apr 2, 2020

Uh oh!

benjaminapetersen commented Apr 2, 2020

Uh oh!

openshift-cherrypick-robot commented Apr 2, 2020

Uh oh!

openshift-ci-robot commented Apr 16, 2020

Uh oh!

benjaminapetersen commented Apr 16, 2020

Uh oh!

openshift-ci-robot commented Apr 16, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants