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

Bug 1980411: [release-4.8] pkg/cvo/egress: Load HTTPS proxy from Proxy status #627

Merged
merged 1 commit into from Sep 10, 2021

Conversation

palonsoro
Copy link

@palonsoro palonsoro commented Jul 8, 2021

Backport to release-4.8 of PR 621

Since 4.2's ea5e3bc (Add http transport for cincinnati to enable
proxy, 2019-07-16, #219), the CVO has been loading proxy config from
the spec property. We should be loading from status instead, so we
benefit from the network operator's validation. Risk is small,
because unlike some other in-cluster components, the CVO is unlikely
to break things if it is temporarily consuming a broken proxy
configuration.

This is similar to c9fab43 (pkg/cvo: Fetch proxy CA certs from
openshift-config-managed/trusted-ca-bundle, 2020-01-31, #311), where
we moved our trusted CA source from the user-configured ConfigMap to
the network-operator-validated ConfigMap.

Since 4.2's ea5e3bc (Add http transport for cincinnati to enable
proxy, 2019-07-16, openshift#219), the CVO has been loading proxy config from
the spec property.  We should be loading from status instead, so we
benefit from the network operator's validation.  Risk is small,
because unlike some other in-cluster components, the CVO is unlikely
to break things if it is temporarily consuming a broken proxy
configuration.

This is similar to c9fab43 (pkg/cvo: Fetch proxy CA certs from
openshift-config-managed/trusted-ca-bundle, 2020-01-31, openshift#311), where
we moved our trusted CA source from the user-configured ConfigMap to
the network-operator-validated ConfigMap.
@openshift-ci openshift-ci bot added the bugzilla/severity-low Referenced Bugzilla bug's severity is low for the branch this PR is targeting. label Jul 8, 2021
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jul 8, 2021

@palonsoro: This pull request references Bugzilla bug 1980411, which is invalid:

  • expected the bug to target the "4.8.0" release, but it targets "4.8.z" 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 1980411: [release-4.8] pkg/cvo/egress: Load HTTPS proxy from Proxy status

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 openshift-ci bot added the bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. label Jul 8, 2021
@openshift-ci openshift-ci bot requested review from jottofar and sdodson July 8, 2021 15:34
@palonsoro
Copy link
Author

/assign @sdodson

@sdodson
Copy link
Member

sdodson commented Jul 8, 2021

/uncc
/cc @wking
Jack and Trevor can review.

@openshift-ci openshift-ci bot requested review from wking and removed request for sdodson July 8, 2021 15:36
@palonsoro
Copy link
Author

Ok. Sorry, I did it because the bot suggested so

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jul 8, 2021

@palonsoro: This pull request references Bugzilla bug 1980411, which is invalid:

  • expected the bug to target the "4.8.0" release, but it targets "4.8.z" 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 1980411: [release-4.8] pkg/cvo/egress: Load HTTPS proxy from Proxy status

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.

@palonsoro
Copy link
Author

/retest

@jianlinliu
Copy link

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jul 13, 2021
@jianlinliu
Copy link

/label qe-approved

@openshift-ci openshift-ci bot added the qe-approved Signifies that QE has signed off on this PR label Jul 13, 2021
@jianlinliu
Copy link

/bugzilla cc-qa

Steps:

[root@preserve-jialiu-ansible ~]# oc get co network
NAME      VERSION                                                  AVAILABLE   PROGRESSING   DEGRADED   SINCE
network   4.8.0-0.ci.test-2021-07-13-030431-ci-ln-ny1cyfk-latest   True        False         False      48m
[root@preserve-jialiu-ansible ~]# oc edit proxies.config.openshift.io
proxy.config.openshift.io/cluster edited
[root@preserve-jialiu-ansible ~]# oc get proxies.config.openshift.io cluster -o json
{
    "apiVersion": "config.openshift.io/v1",
    "kind": "Proxy",
    "metadata": {
        "creationTimestamp": "2021-07-13T05:29:08Z",
        "generation": 2,
        "name": "cluster",
        "resourceVersion": "40145",
        "uid": "7c6476f1-3362-45ca-9df8-e975848cd663"
    },
    "spec": {
        "httpProxy": "proxy-user1",
        "httpsProxy": "proxy-user1",
        "noProxy": "test.no-proxy.com",
        "trustedCA": {
            "name": ""
        }
    },
    "status": {
        "httpProxy": "http://proxy-user1:JYgU8qRZV4DY4PXJbxJK@10.0.0.2:3128",
        "httpsProxy": "http://proxy-user1:JYgU8qRZV4DY4PXJbxJK@10.0.0.2:3128",
        "noProxy": ".cluster.local,.svc,10.0.0.0/16,10.128.0.0/14,127.0.0.1,169.254.169.254,172.30.0.0/16,api-int.jialiu1980411.qe.gcp.devcluster.openshift.com,localhost,metadata,metadata.google.internal,metadata.google.internal.,test.no-proxy.com"
    }
}
[root@preserve-jialiu-ansible ~]#  oc describe co network
Name:         network
Namespace:    
Labels:       <none>
Annotations:  include.release.openshift.io/ibm-cloud-managed: true
              include.release.openshift.io/self-managed-high-availability: true
              include.release.openshift.io/single-node-developer: true
              network.operator.openshift.io/last-seen-state: {"DaemonsetStates":[],"DeploymentStates":[]}
API Version:  config.openshift.io/v1
Kind:         ClusterOperator
Metadata:
  Creation Timestamp:  2021-07-13T05:29:13Z
  Generation:          1
  Managed Fields:
    API Version:  config.openshift.io/v1
    Fields Type:  FieldsV1
    fieldsV1:
      f:metadata:
        f:annotations:
          .:
          f:include.release.openshift.io/ibm-cloud-managed:
          f:include.release.openshift.io/self-managed-high-availability:
          f:include.release.openshift.io/single-node-developer:
      f:spec:
      f:status:
        .:
        f:extension:
    Manager:      cluster-version-operator
    Operation:    Update
    Time:         2021-07-13T05:29:14Z
    API Version:  config.openshift.io/v1
    Fields Type:  FieldsV1
    fieldsV1:
      f:metadata:
        f:annotations:
          f:network.operator.openshift.io/last-seen-state:
      f:status:
        f:conditions:
        f:relatedObjects:
        f:versions:
    Manager:         cluster-network-operator
    Operation:       Update
    Time:            2021-07-13T05:35:21Z
  Resource Version:  40147
  UID:               bae49eb1-723c-44ff-a8d7-62e0f08f1d1e
Spec:
Status:
  Conditions:
    Last Transition Time:  2021-07-13T06:24:53Z
    Message:               The configuration is invalid for proxy 'cluster' (invalid httpProxy URI: parse "proxy-user1": invalid URI for request). Use 'oc edit proxy.config.openshift.io cluster' to fix.
    Reason:                InvalidProxyConfig
    Status:                True
    Type:                  Degraded
    Last Transition Time:  2021-07-13T05:34:27Z
    Status:                False
    Type:                  ManagementStateDegraded
    Last Transition Time:  2021-07-13T05:34:27Z
    Status:                True
    Type:                  Upgradeable
<--snip-->
[root@preserve-jialiu-ansible ~]# oc get co network
NAME      VERSION                                                  AVAILABLE   PROGRESSING   DEGRADED   SINCE
network   4.8.0-0.ci.test-2021-07-13-030431-ci-ln-ny1cyfk-latest   True        False         True       52m
[root@preserve-jialiu-ansible ~]# oc adm upgrade
Cluster version is 4.8.0-0.ci.test-2021-07-13-030431-ci-ln-ny1cyfk-latest

warning: Cannot display available updates:
  Reason: VersionNotFound
  Message: Unable to retrieve available updates: currently reconciling cluster version 4.8.0-0.ci.test-2021-07-13-030431-ci-ln-ny1cyfk-latest not found in the "stable-4.8" channel

It does not complain about the proxy URI, using staus for that.

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jul 13, 2021

@jianlinliu: This pull request references Bugzilla bug 1980411, which is invalid:

  • expected the bug to target the "4.8.0" release, but it targets "4.8.z" 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:

/bugzilla cc-qa

Steps:

[root@preserve-jialiu-ansible ~]# oc get co network
NAME      VERSION                                                  AVAILABLE   PROGRESSING   DEGRADED   SINCE
network   4.8.0-0.ci.test-2021-07-13-030431-ci-ln-ny1cyfk-latest   True        False         False      48m
[root@preserve-jialiu-ansible ~]# oc edit proxies.config.openshift.io
proxy.config.openshift.io/cluster edited
[root@preserve-jialiu-ansible ~]# oc get proxies.config.openshift.io cluster -o json
{
   "apiVersion": "config.openshift.io/v1",
   "kind": "Proxy",
   "metadata": {
       "creationTimestamp": "2021-07-13T05:29:08Z",
       "generation": 2,
       "name": "cluster",
       "resourceVersion": "40145",
       "uid": "7c6476f1-3362-45ca-9df8-e975848cd663"
   },
   "spec": {
       "httpProxy": "proxy-user1",
       "httpsProxy": "proxy-user1",
       "noProxy": "test.no-proxy.com",
       "trustedCA": {
           "name": ""
       }
   },
   "status": {
       "httpProxy": "http://proxy-user1:JYgU8qRZV4DY4PXJbxJK@10.0.0.2:3128",
       "httpsProxy": "http://proxy-user1:JYgU8qRZV4DY4PXJbxJK@10.0.0.2:3128",
       "noProxy": ".cluster.local,.svc,10.0.0.0/16,10.128.0.0/14,127.0.0.1,169.254.169.254,172.30.0.0/16,api-int.jialiu1980411.qe.gcp.devcluster.openshift.com,localhost,metadata,metadata.google.internal,metadata.google.internal.,test.no-proxy.com"
   }
}
[root@preserve-jialiu-ansible ~]#  oc describe co network
Name:         network
Namespace:    
Labels:       <none>
Annotations:  include.release.openshift.io/ibm-cloud-managed: true
             include.release.openshift.io/self-managed-high-availability: true
             include.release.openshift.io/single-node-developer: true
             network.operator.openshift.io/last-seen-state: {"DaemonsetStates":[],"DeploymentStates":[]}
API Version:  config.openshift.io/v1
Kind:         ClusterOperator
Metadata:
 Creation Timestamp:  2021-07-13T05:29:13Z
 Generation:          1
 Managed Fields:
   API Version:  config.openshift.io/v1
   Fields Type:  FieldsV1
   fieldsV1:
     f:metadata:
       f:annotations:
         .:
         f:include.release.openshift.io/ibm-cloud-managed:
         f:include.release.openshift.io/self-managed-high-availability:
         f:include.release.openshift.io/single-node-developer:
     f:spec:
     f:status:
       .:
       f:extension:
   Manager:      cluster-version-operator
   Operation:    Update
   Time:         2021-07-13T05:29:14Z
   API Version:  config.openshift.io/v1
   Fields Type:  FieldsV1
   fieldsV1:
     f:metadata:
       f:annotations:
         f:network.operator.openshift.io/last-seen-state:
     f:status:
       f:conditions:
       f:relatedObjects:
       f:versions:
   Manager:         cluster-network-operator
   Operation:       Update
   Time:            2021-07-13T05:35:21Z
 Resource Version:  40147
 UID:               bae49eb1-723c-44ff-a8d7-62e0f08f1d1e
Spec:
Status:
 Conditions:
   Last Transition Time:  2021-07-13T06:24:53Z
   Message:               The configuration is invalid for proxy 'cluster' (invalid httpProxy URI: parse "proxy-user1": invalid URI for request). Use 'oc edit proxy.config.openshift.io cluster' to fix.
   Reason:                InvalidProxyConfig
   Status:                True
   Type:                  Degraded
   Last Transition Time:  2021-07-13T05:34:27Z
   Status:                False
   Type:                  ManagementStateDegraded
   Last Transition Time:  2021-07-13T05:34:27Z
   Status:                True
   Type:                  Upgradeable
<--snip-->
[root@preserve-jialiu-ansible ~]# oc get co network
NAME      VERSION                                                  AVAILABLE   PROGRESSING   DEGRADED   SINCE
network   4.8.0-0.ci.test-2021-07-13-030431-ci-ln-ny1cyfk-latest   True        False         True       52m
[root@preserve-jialiu-ansible ~]# oc adm upgrade
Cluster version is 4.8.0-0.ci.test-2021-07-13-030431-ci-ln-ny1cyfk-latest

warning: Cannot display available updates:
 Reason: VersionNotFound
 Message: Unable to retrieve available updates: currently reconciling cluster version 4.8.0-0.ci.test-2021-07-13-030431-ci-ln-ny1cyfk-latest not found in the "stable-4.8" channel

It does not complain about the proxy URI, using staus for that.

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-bot
Copy link
Contributor

/bugzilla refresh

Recalculating validity in case the underlying Bugzilla bug has changed.

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jul 14, 2021

@openshift-bot: This pull request references Bugzilla bug 1980411, which is invalid:

  • expected the bug to target the "4.8.0" release, but it targets "4.8.z" 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:

/bugzilla refresh

Recalculating validity in case the underlying Bugzilla bug has changed.

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-bot
Copy link
Contributor

/bugzilla refresh

Recalculating validity in case the underlying Bugzilla bug has changed.

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jul 15, 2021

@openshift-bot: This pull request references Bugzilla bug 1980411, which is invalid:

  • expected the bug to target the "4.8.0" release, but it targets "4.8.z" 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:

/bugzilla refresh

Recalculating validity in case the underlying Bugzilla bug has changed.

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-bot
Copy link
Contributor

/bugzilla refresh

Recalculating validity in case the underlying Bugzilla bug has changed.

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jul 18, 2021

@openshift-bot: This pull request references Bugzilla bug 1980411, which is invalid:

  • expected the bug to target the "4.8.0" release, but it targets "4.8.z" 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:

/bugzilla refresh

Recalculating validity in case the underlying Bugzilla bug has changed.

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-bot
Copy link
Contributor

/bugzilla refresh

Recalculating validity in case the underlying Bugzilla bug has changed.

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jul 19, 2021

@openshift-bot: This pull request references Bugzilla bug 1980411, which is invalid:

  • expected the bug to target the "4.8.0" release, but it targets "4.8.z" 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:

/bugzilla refresh

Recalculating validity in case the underlying Bugzilla bug has changed.

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-bot
Copy link
Contributor

/bugzilla refresh

Recalculating validity in case the underlying Bugzilla bug has changed.

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jul 20, 2021

@openshift-bot: This pull request references Bugzilla bug 1980411, which is invalid:

  • expected the bug to target the "4.8.0" release, but it targets "4.8.z" 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:

/bugzilla refresh

Recalculating validity in case the underlying Bugzilla bug has changed.

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-bot
Copy link
Contributor

/bugzilla refresh

Recalculating validity in case the underlying Bugzilla bug has changed.

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jul 21, 2021

@openshift-bot: This pull request references Bugzilla bug 1980411, which is invalid:

  • expected the bug to target the "4.8.0" release, but it targets "4.8.z" 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:

/bugzilla refresh

Recalculating validity in case the underlying Bugzilla bug has changed.

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-bot
Copy link
Contributor

/bugzilla refresh

Recalculating validity in case the underlying Bugzilla bug has changed.

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jul 22, 2021

@openshift-bot: This pull request references Bugzilla bug 1980411, which is invalid:

  • expected the bug to target the "4.8.0" release, but it targets "4.8.z" 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:

/bugzilla refresh

Recalculating validity in case the underlying Bugzilla bug has changed.

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.

@sdodson
Copy link
Member

sdodson commented Jul 22, 2021

/bugzilla refresh

@openshift-ci openshift-ci bot added the bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. label Jul 22, 2021
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jul 22, 2021

@sdodson: This pull request references Bugzilla bug 1980411, which is valid.

6 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target release (4.8.z) matches configured target release for branch (4.8.z)
  • bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)
  • dependent bug Bugzilla bug 1978774 is in the state VERIFIED, which is one of the valid states (VERIFIED, RELEASE_PENDING, CLOSED (ERRATA), CLOSED (CURRENTRELEASE))
  • dependent Bugzilla bug 1978774 targets the "4.9.0" release, which is one of the valid target releases: 4.9.0
  • bug has dependents

Requesting review from QA contact:
/cc @jianlinliu

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.

@openshift-ci openshift-ci bot removed the bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. label Jul 22, 2021
@openshift-ci openshift-ci bot requested a review from jianlinliu July 22, 2021 17:51
Copy link
Member

@wking wking left a comment

Choose a reason for hiding this comment

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

Sorry, this one had fallen off my radar. Looks like it's backporting #621, but not cleanly because it's missing the portion of #604 that touched this code. That's ok with me.

/lgtm

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Aug 25, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jianlinliu, palonsoro, wking

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

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 25, 2021
@mfojtik mfojtik added the cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. label Sep 10, 2021
@mfojtik
Copy link
Member

mfojtik commented Sep 10, 2021

[patch-manager] 🚀 Approved for z-stream by score: 0.10

adding cherry-pick approved

@openshift-merge-robot openshift-merge-robot merged commit 011016c into openshift:release-4.8 Sep 10, 2021
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Sep 10, 2021

@palonsoro: All pull requests linked via external trackers have merged:

Bugzilla bug 1980411 has been moved to the MODIFIED state.

In response to this:

Bug 1980411: [release-4.8] pkg/cvo/egress: Load HTTPS proxy from Proxy status

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.

@palonsoro
Copy link
Author

/cherry-pick release-4.7

@openshift-cherrypick-robot

@palonsoro: new pull request created: #664

In response to this:

/cherry-pick release-4.7

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.

@palonsoro
Copy link
Author

/cherry-pick release-4.6

@openshift-cherrypick-robot

@palonsoro: new pull request created: #673

In response to this:

/cherry-pick release-4.6

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. bugzilla/severity-low Referenced Bugzilla bug's severity is low for the branch this PR is targeting. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. lgtm Indicates that a PR is ready to be merged. qe-approved Signifies that QE has signed off on this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants