Skip to content

Conversation

@Miciah
Copy link
Contributor

@Miciah Miciah commented Apr 26, 2023

This is a manual cherry-pick of #803. The changes include an openshift/api bump.


Bump openshift/api for PROXY protocol for Private

Bump to github.com/openshift/api@398424d53f7473dcc8be52f78692d2bea9b46b2d to get the "Protocol" field in the "Private" endpoint publishing strategy parameters to allow configuring PROXY protocol.

  • go.mod: Update.
  • go.sum:
  • manifests/00-custom-resource-definition.yaml:
  • pkg/manifests/bindata.go:
  • vendor/github.com/openshift/api/*:
  • vendor/modules.txt: Regenerate.

Allow PROXY protocol for "Private" strategy

Allow the user to configure an IngressController with the "Private" endpoint publishing strategy type to use PROXY protocol.

  • pkg/operator/controller/ingress/controller.go (setDefaultPublishingStrategy): Check whether the user specified the protocol on the "Private" endpoint publishing strategy parameters, and update status as necessary.
    (IsProxyProtocolNeeded): Check whether the user specified the protocol on the "Private" endpoint publishing strategy parameters.
  • pkg/operator/controller/ingress/controller_test.go (TestSetDefaultPublishingStrategyHandlesUpdates, TestIsProxyProtocolNeeded): Add test cases for the "Private" endpoint publishing strategy.

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Apr 26, 2023

@Miciah: No Bugzilla bug is referenced in the title of this pull request.
To reference a bug, add 'Bug XXX:' to the title of this pull request and request another bug refresh with /bugzilla refresh.

Details

In response to this:

OCPBUGS-3560: Allow PROXY protocol for "Private" strategy

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 openshift-ci-robot added jira/severity-moderate Referenced Jira bug's severity is moderate for the branch this PR is targeting. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. jira/valid-bug Indicates that a referenced Jira bug is valid 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. labels Apr 26, 2023
@openshift-ci-robot
Copy link
Contributor

@Miciah: This pull request references Jira Issue OCPBUGS-3560, which is valid. The bug has been moved to the POST state.

6 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target version (4.11.z) matches configured target version for branch (4.11.z)
  • bug is in the state ASSIGNED, which is one of the valid states (NEW, ASSIGNED, POST)
  • dependent bug Jira Issue OCPBUGS-3559 is in the state Closed (Done), which is one of the valid states (VERIFIED, RELEASE PENDING, CLOSED (ERRATA), CLOSED (CURRENT RELEASE), CLOSED (DONE))
  • dependent Jira Issue OCPBUGS-3559 targets the "4.12.0" version, which is one of the valid target versions: 4.12.0, 4.12.z
  • bug has dependents

Requesting review from QA contact:
/cc @ShudiLi

The bug has been updated to refer to the pull request using the external bug tracker.

Details

In response to this:

This is a manual cherry-pick of #803. The changes include an openshift/api bump.


Bump openshift/api for PROXY protocol for Private

Bump to github.com/openshift/api@398424d53f7473dcc8be52f78692d2bea9b46b2d to get the "Protocol" field in the "Private" endpoint publishing strategy parameters to allow configuring PROXY protocol.

  • go.mod: Update.
  • go.sum:
  • manifests/00-custom-resource-definition.yaml:
  • pkg/manifests/bindata.go:
  • vendor/github.com/openshift/api/*:
  • vendor/modules.txt: Regenerate.

Allow PROXY protocol for "Private" strategy

Allow the user to configure an IngressController with the "Private" endpoint publishing strategy type to use PROXY protocol.

  • pkg/operator/controller/ingress/controller.go (setDefaultPublishingStrategy): Check whether the user specified the protocol on the "Private" endpoint publishing strategy parameters, and update status as necessary.
    (IsProxyProtocolNeeded): Check whether the user specified the protocol on the "Private" endpoint publishing strategy parameters.
  • pkg/operator/controller/ingress/controller_test.go (TestSetDefaultPublishingStrategyHandlesUpdates, TestIsProxyProtocolNeeded): Add test cases for the "Private" endpoint publishing strategy.

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 requested review from ShudiLi, candita and gcs278 April 26, 2023 13:36
@Miciah Miciah changed the title OCPBUGS-3560: Allow PROXY protocol for "Private" strategy [release-4.11] OCPBUGS-3560: Allow PROXY protocol for "Private" strategy Apr 26, 2023
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Apr 26, 2023

@Miciah: No Bugzilla bug is referenced in the title of this pull request.
To reference a bug, add 'Bug XXX:' to the title of this pull request and request another bug refresh with /bugzilla refresh.

Retaining the bugzilla/valid-bug label as it was manually added.

Details

In response to this:

[release-4.11] OCPBUGS-3560: Allow PROXY protocol for "Private" strategy

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.

@candita
Copy link
Contributor

candita commented Apr 26, 2023

/assign @gcs278

@ShudiLi
Copy link
Member

ShudiLi commented Apr 27, 2023

From QE side, tested it with 4.11.0-0.ci.test-2023-04-27-005005-ci-ln-hbmijl2-latest
1.
% oc get clusterversion
NAME VERSION AVAILABLE PROGRESSING SINCE STATUS
version 4.11.0-0.ci.test-2023-04-27-005005-ci-ln-hbmijl2-latest True False 19m Cluster version is 4.11.0-0.ci.test-2023-04-27-005005-ci-ln-hbmijl2-latest
%

  1. check the status.endpointPublishingStrategy, expect enable the proxy: {"private":{"protocol":"PROXY"},"type":"Private"}
    % oc -n openshift-ingress-operator get ingresscontrollers/int1 -o 'jsonpath={.status.endpointPublishingStrategy}'
    {"private":{"protocol":"PROXY"},"type":"Private"}
    %

  2. check the env ROUTER_USE_PROXY_PROTOCOL, expect {"name":"ROUTER_USE_PROXY_PROTOCOL","value":"true"}
    % oc -n openshift-ingress get deployments/router-int1 -o 'jsonpath={.spec.template.spec.containers[0].env[?(@.name=="ROUTER_USE_PROXY_PROTOCOL")]}'
    {"name":"ROUTER_USE_PROXY_PROTOCOL","value":"true"}% %

/label qe-approved
thanks %

@openshift-ci openshift-ci bot added the qe-approved Signifies that QE has signed off on this PR label Apr 27, 2023
// img/
// a.png
// b.png
//
Copy link
Contributor

Choose a reason for hiding this comment

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

This is why ci/prow/verify is failing. It was spaces and now is indents. Is this a go version issue as you mentioned here: #869 (comment)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Miciah Miciah force-pushed the cherry-pick-803-to-release-4.11 branch from ba9df6b to be7ae90 Compare April 27, 2023 20:30
@gcs278
Copy link
Contributor

gcs278 commented Apr 27, 2023

I'm the only reviewer so
/approve
/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Apr 27, 2023
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Apr 27, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: gcs278

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:

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 Apr 27, 2023
Miciah added 2 commits April 27, 2023 16:42
Bump to github.com/openshift/api@398424d53f7473dcc8be52f78692d2bea9b46b2d
to get the "Protocol" field in the "Private" endpoint publishing strategy
parameters to allow configuring PROXY protocol.

* go.mod: Update.
* go.sum:
* manifests/00-custom-resource-definition.yaml:
* pkg/manifests/bindata.go:
* vendor/github.com/openshift/api/*:
* vendor/modules.txt: Regenerate.
Allow the user to configure an IngressController with the "Private"
endpoint publishing strategy type to use PROXY protocol.

This commit fixes bug 2104481.

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

* pkg/operator/controller/ingress/controller.go
(setDefaultPublishingStrategy): Check whether the user specified the
protocol on the "Private" endpoint publishing strategy parameters, and
update status as necessary.
(IsProxyProtocolNeeded): Check whether the user specified the protocol on
the "Private" endpoint publishing strategy parameters.
* pkg/operator/controller/ingress/controller_test.go
(TestSetDefaultPublishingStrategyHandlesUpdates)
(TestIsProxyProtocolNeeded): Add test cases for the "Private" endpoint
publishing strategy.
@Miciah Miciah force-pushed the cherry-pick-803-to-release-4.11 branch from be7ae90 to 2fc2f3b Compare April 27, 2023 20:42
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Apr 27, 2023
@Miciah
Copy link
Contributor Author

Miciah commented Apr 27, 2023

verify failed because I had neglected to git add vendor/github.com/openshift/api/models-schema after go mod vendor. https://github.com/openshift/cluster-ingress-operator/compare/be7ae90c7363152360fc01a913def9c21d49dc62..2fc2f3b16f9777160972b5b23bc51cd3384cd518 adds the missing file.

@Miciah
Copy link
Contributor Author

Miciah commented Apr 28, 2023

e2e-aws-operator failed because TestHostNetworkPortBinding failed:

=== RUN   TestAll/serial/TestHostNetworkPortBinding
=== PAUSE TestAll/serial/TestHostNetworkPortBinding
=== CONT  TestAll/serial/TestHostNetworkPortBinding
    operator_test.go:1009: Expected conditions: map[Admitted:True Available:True DNSManaged:False DeploymentReplicasAllAvailable:True LoadBalancerManaged:False]
         Current conditions: map[Admitted:True Available:True DNSManaged:False Degraded:False DeploymentAvailable:True DeploymentReplicasAllAvailable:False DeploymentReplicasMinAvailable:True LoadBalancerManaged:False PodsScheduled:False Progressing:False Upgradeable:True]
    operator_test.go:1011: failed to observe expected conditions for the second ingresscontroller: timed out waiting for the condition
    operator_test.go:1034: deleted ingresscontroller samehost
    operator_test.go:1034: deleted ingresscontroller hostnetworkportbinding

From events.json, it seems that the "samehost" router pod got deleted and failed to be rescheduled:

% jq -rc < events.json '.items|sort_by(.metadata.creationTimestamp)|.[]|select(.involvedObject.name//""|contains("samehost"))|.metadata.creationTimestamp+" ("+(.count//1|tostring)+"x) "+.reason+": "+.message' 
2023-04-27T21:36:48Z (1x) FailedToCreateEndpoint: Failed to create endpoint for service openshift-ingress/router-internal-samehost: endpoints "router-internal-samehost" already exists
2023-04-27T21:36:48Z (1x) Admitted: ingresscontroller passed validation
2023-04-27T21:36:48Z (2x) CreatedDefaultCertificate: Created default wildcard certificate "router-certs-samehost"
2023-04-27T21:36:48Z (1x) Scheduled: Successfully assigned openshift-ingress/router-samehost-54dbc7dd5f-9vxhj to ip-10-0-136-46.us-east-2.compute.internal by ip-10-0-228-168
2023-04-27T21:36:48Z (1x) FailedMount: MountVolume.SetUp failed for volume "metrics-certs" : secret "router-metrics-certs-samehost" not found
2023-04-27T21:36:48Z (1x) FailedMount: MountVolume.SetUp failed for volume "default-certificate" : secret "router-certs-samehost" not found
2023-04-27T21:36:48Z (1x) SuccessfulCreate: Created pod: router-samehost-54dbc7dd5f-9vxhj
2023-04-27T21:36:48Z (1x) ScalingReplicaSet: Scaled up replica set router-samehost-54dbc7dd5f to 1
2023-04-27T21:36:49Z (1x) Pulled: Container image "registry.build01.ci.openshift.org/ci-op-bibmbvxp/stable@sha256:b477ca8dbeaa1c77f6dfa3af5e8eaf36cf25ae8b9acc2ccc1cc48bf2c89077f7" already present on machine
2023-04-27T21:36:49Z (1x) Created: Created container router
2023-04-27T21:36:49Z (1x) Started: Started container router
2023-04-27T21:37:18Z (1x) Killing: Stopping container router
2023-04-27T21:37:18Z (1x) FailedScheduling: 0/6 nodes are available: 1 node(s) were unschedulable, 2 node(s) didn't match Pod's node affinity/selector, 3 node(s) had untolerated taint {node-role.kubernetes.io/master: }. preemption: 0/6 nodes are available: 6 Preemption is not helpful for scheduling.
2023-04-27T21:37:18Z (1x) SuccessfulCreate: Created pod: router-samehost-54dbc7dd5f-rtsp6
2023-04-27T21:37:23Z (5x) ProbeError: Readiness probe error: HTTP probe failed with statuscode: 500
body: [+]backend-http ok
[+]has-synced ok
[-]process-running failed: reason withheld
healthz check failed


2023-04-27T21:37:23Z (5x) Unhealthy: Readiness probe failed: HTTP probe failed with statuscode: 500
2023-04-27T21:39:39Z (1x) FailedScheduling: 0/6 nodes are available: 2 node(s) didn't match Pod's node affinity/selector, 2 node(s) had untolerated taint {node-role.kubernetes.io/master: }, 2 node(s) were unschedulable. preemption: 0/6 nodes are available: 6 Preemption is not helpful for scheduling.
2023-04-27T21:41:48Z (3x) DeletedDeployment: Deleted deployment openshift-ingress/router-samehost
2023-04-27T21:41:48Z (1x) FailedScheduling: skip schedule deleting pod: openshift-ingress/router-samehost-54dbc7dd5f-rtsp6

It looks like nodes are getting rebooted while the tests are running, which could explain why the "samehost" router pod was deleted and couldn't be rescheduled:

%  jq -rc < events.json '.items|sort_by(.metadata.creationTimestamp)|.[]|select(.involvedObject.kind=="Node")|.metadata.creationTimestamp+" ("+(.count//1|tostring)+"x) "+.involvedObject.name+" "+.reason+": "+.message'                                                                                                                                                                                                                
[...]
2023-04-27T21:36:56Z (1x) ip-10-0-178-19.us-east-2.compute.internal Rebooted: Node ip-10-0-178-19.us-east-2.compute.internal has been rebooted, boot id: d8e11df6-1669-43f6-96cd-cb0fd63b0890                                                                                                                                                                                                                                           
2023-04-27T21:36:56Z (1x) ip-10-0-178-19.us-east-2.compute.internal NodeReady: Node ip-10-0-178-19.us-east-2.compute.internal status is now: NodeReady                                                                                                                                                                                                                                                                                  
2023-04-27T21:36:56Z (1x) ip-10-0-178-19.us-east-2.compute.internal NodeNotSchedulable: Node ip-10-0-178-19.us-east-2.compute.internal status is now: NodeNotSchedulable                                                                                                                                                                                                                                                                
2023-04-27T21:37:01Z (1x) ip-10-0-178-19.us-east-2.compute.internal Starting: openshift-sdn done initializing node networking.                                                                                                                                                                                                                                                                                                          
2023-04-27T21:37:05Z (1x) ip-10-0-182-216.us-east-2.compute.internal Starting: openshift-sdn done initializing node networking.                                                                                                                                                                                                                                                                                                         
2023-04-27T21:37:06Z (1x) ip-10-0-182-216.us-east-2.compute.internal FastControllerResync: Controller "CheckEndpointsTimeToStart" resync interval is set to 0s which might lead to client request throttling                                                                                                                                                                                                                            
2023-04-27T21:37:06Z (1x) ip-10-0-182-216.us-east-2.compute.internal FastControllerResync: Controller "CheckEndpointsStop" resync interval is set to 0s which might lead to client request throttling                                                                                                                                                                                                                                   
2023-04-27T21:37:12Z (1x) ip-10-0-178-19.us-east-2.compute.internal Uncordon: Update completed for config rendered-worker-7b7e40488c1dad43017388e8822faf7f and node has been uncordoned                                                                                                                                                                                                                                                 
2023-04-27T21:37:12Z (1x) ip-10-0-178-19.us-east-2.compute.internal NodeDone: Setting node ip-10-0-178-19.us-east-2.compute.internal, currentConfig rendered-worker-7b7e40488c1dad43017388e8822faf7f to Done                                                                                                                                                                                                                            
2023-04-27T21:37:12Z (1x) ip-10-0-178-19.us-east-2.compute.internal ConfigDriftMonitorStarted: Config Drift Monitor started, watching against rendered-worker-7b7e40488c1dad43017388e8822faf7f                                                                                                                                                                                                                                          
2023-04-27T21:37:13Z (1x) ip-10-0-136-46.us-east-2.compute.internal ConfigDriftMonitorStopped: Config Drift Monitor stopped                                                                                                                                                                                                                                                                                                             
2023-04-27T21:37:13Z (1x) ip-10-0-136-46.us-east-2.compute.internal Cordon: Cordoned node to apply update                                                                                                                                                                                                                                                                                                                               
2023-04-27T21:37:13Z (1x) ip-10-0-136-46.us-east-2.compute.internal Drain: Draining node to update config.                                                                                                                                                                                                                                                                                                                              
2023-04-27T21:37:16Z (1x) ip-10-0-178-19.us-east-2.compute.internal NodeSchedulable: Node ip-10-0-178-19.us-east-2.compute.internal status is now: NodeSchedulable                                                                                                                                                                                                                                                                      
2023-04-27T21:37:18Z (1x) ip-10-0-182-216.us-east-2.compute.internal Uncordon: Update completed for config rendered-master-8a8418bdac10dea73efedbf2a4276f74 and node has been uncordoned                                                                                                                                                                                                                                                
2023-04-27T21:37:18Z (1x) ip-10-0-182-216.us-east-2.compute.internal NodeDone: Setting node ip-10-0-182-216.us-east-2.compute.internal, currentConfig rendered-master-8a8418bdac10dea73efedbf2a4276f74 to Done                                                                                                                                                                                                                          
[...]

I wouldn't expect nodes to reboot during E2E tests, but anyway, it doesn't look like a problem with cluster-ingress-operator or with the test itself.
/test e2e-aws-operator

@Miciah
Copy link
Contributor Author

Miciah commented Apr 28, 2023

e2e-aws-operator failed because the kube-apiserver, kube-controller-manager, kube-scheduler, and network clusteroperators failed to roll out updates.
/test e2e-aws-operator

@gcs278
Copy link
Contributor

gcs278 commented Apr 28, 2023

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Apr 28, 2023
@Miciah
Copy link
Contributor Author

Miciah commented Apr 28, 2023

e2e-aws-operator failed because many clusteroperators failed to roll out all their pods.
/test e2e-aws-operator

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Apr 28, 2023

@Miciah: all tests passed!

Full PR test history. Your PR dashboard.

Details

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.

@Miciah
Copy link
Contributor Author

Miciah commented Apr 28, 2023

/label backport-risk-assessed

@openshift-ci openshift-ci bot added the backport-risk-assessed Indicates a PR to a release branch has been evaluated and considered safe to accept. label Apr 28, 2023
@ShudiLi
Copy link
Member

ShudiLi commented May 4, 2023

/label cherry-pick-approved
thanks

@openshift-ci openshift-ci bot added the cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. label May 4, 2023
@openshift-merge-robot openshift-merge-robot merged commit ce29f22 into openshift:release-4.11 May 4, 2023
@openshift-ci-robot
Copy link
Contributor

@Miciah: Jira Issue OCPBUGS-3560: All pull requests linked via external trackers have merged:

Jira Issue OCPBUGS-3560 has been moved to the MODIFIED state.

Details

In response to this:

This is a manual cherry-pick of #803. The changes include an openshift/api bump.


Bump openshift/api for PROXY protocol for Private

Bump to github.com/openshift/api@398424d53f7473dcc8be52f78692d2bea9b46b2d to get the "Protocol" field in the "Private" endpoint publishing strategy parameters to allow configuring PROXY protocol.

  • go.mod: Update.
  • go.sum:
  • manifests/00-custom-resource-definition.yaml:
  • pkg/manifests/bindata.go:
  • vendor/github.com/openshift/api/*:
  • vendor/modules.txt: Regenerate.

Allow PROXY protocol for "Private" strategy

Allow the user to configure an IngressController with the "Private" endpoint publishing strategy type to use PROXY protocol.

  • pkg/operator/controller/ingress/controller.go (setDefaultPublishingStrategy): Check whether the user specified the protocol on the "Private" endpoint publishing strategy parameters, and update status as necessary.
    (IsProxyProtocolNeeded): Check whether the user specified the protocol on the "Private" endpoint publishing strategy parameters.
  • pkg/operator/controller/ingress/controller_test.go (TestSetDefaultPublishingStrategyHandlesUpdates, TestIsProxyProtocolNeeded): Add test cases for the "Private" endpoint publishing strategy.

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

[ART PR BUILD NOTIFIER]

This PR has been included in build ose-cluster-ingress-operator-container-v4.11.0-202305040215.p0.gce29f22.assembly.stream for distgit ose-cluster-ingress-operator.
All builds following this will include this PR.

@openshift-merge-robot
Copy link
Contributor

Fix included in accepted release 4.11.0-0.nightly-2023-05-04-100517

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. backport-risk-assessed Indicates a PR to a release branch has been evaluated and considered safe to accept. 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. jira/severity-moderate Referenced Jira bug's severity is moderate for the branch this PR is targeting. jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. 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.

9 participants