Skip to content

Conversation

@Miciah
Copy link
Contributor

@Miciah Miciah commented Jun 15, 2023

Set spec.template.spec.containers[*].ports[*].hostPort on a router deployment if the deployment sets spec.template.spec.hostNetwork to true.

Before this change, the operator left the hostPort field unspecified. As a result, the API set a default value for hostPort. The operator detected this as an external update and attempted to revert it. The change in this PR prevents the operator from making these spurious updates in response to API defaulting.

  • pkg/operator/controller/ingress/deployment.go (desiredRouterDeployment): Set each container port's HostPort to the port's ContainerPort value when HostNetwork is enabled for the deployment.
  • pkg/operator/controller/ingress/deployment_test.go (checkContainerPort): Verify that HostPort is set to ContainerPort if HostNetwork is enabled.

Set spec.template.spec.containers[*].ports[*].hostPort on a router
deployment if the deployment sets spec.template.spec.hostNetwork to true.

Before this commit, the operator left the hostPort field unspecified. As a
result, the API set a default value for hostPort.  The operator detected
this as an external update and attempted to revert it.  This commit
prevents the operator from making these spurious updates in response to API
defaulting.

This commit fixes OCPBUGS-14995.

https://issues.redhat.com/browse/OCPBUGS-14995

* pkg/operator/controller/ingress/deployment.go (desiredRouterDeployment):
Set each container port's HostPort to the port's ContainerPort value when
HostNetwork is enabled for the deployment.
* pkg/operator/controller/ingress/deployment_test.go (checkContainerPort):
Verify that HostPort is set to ContainerPort if HostNetwork is enabled.
@openshift-ci-robot openshift-ci-robot added jira/severity-low Referenced Jira bug's severity is low 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. labels Jun 15, 2023
@openshift-ci-robot
Copy link
Contributor

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

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target version (4.14.0) matches configured target version for branch (4.14.0)
  • bug is in the state New, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @lihongan

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

Details

In response to this:

Set spec.template.spec.containers[*].ports[*].hostPort on a router deployment if the deployment sets spec.template.spec.hostNetwork to true.

Before this change, the operator left the hostPort field unspecified. As a result, the API set a default value for hostPort. The operator detected this as an external update and attempted to revert it. The change in this PR prevents the operator from making these spurious updates in response to API defaulting.

  • pkg/operator/controller/ingress/deployment.go (desiredRouterDeployment): Set each container port's HostPort to the port's ContainerPort value when HostNetwork is enabled for the deployment.
  • pkg/operator/controller/ingress/deployment_test.go (checkContainerPort): Verify that HostPort is set to ContainerPort if HostNetwork is enabled.

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 candita, frobware and lihongan June 15, 2023 02:36
@alebedev87
Copy link
Contributor

/assign

@candita
Copy link
Contributor

candita commented Jun 21, 2023

/assign @alebedev87

@Miciah
Copy link
Contributor Author

Miciah commented Jul 19, 2023

e2e-aws-operator failed because must-gather failed.

@alebedev87
Copy link
Contributor

/lgtm

@alebedev87
Copy link
Contributor

/approve

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

openshift-ci bot commented Aug 3, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alebedev87

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 Aug 3, 2023
@openshift-ci-robot
Copy link
Contributor

/retest-required

Remaining retests: 0 against base HEAD ddba423 and 2 for PR HEAD 58f2a10 in total

@openshift-ci-robot
Copy link
Contributor

/retest-required

Remaining retests: 0 against base HEAD 4e7b2da and 1 for PR HEAD 58f2a10 in total

@openshift-ci-robot
Copy link
Contributor

/retest-required

Remaining retests: 0 against base HEAD 833bc28 and 0 for PR HEAD 58f2a10 in total

@openshift-ci-robot
Copy link
Contributor

/hold

Revision 58f2a10 was retested 3 times: holding

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 5, 2023
@Miciah
Copy link
Contributor Author

Miciah commented Aug 5, 2023

e2e-azure-ovn failed because [bz-Networking][invariant] alert/KubePodNotReady should not be at or above info in ns/openshift-multus failed:

{  KubePodNotReady was at or above info for at least 28s on platformidentification.JobType{Release:"4.14", FromRelease:"", Platform:"azure", Architecture:"amd64", Network:"ovn", Topology:"ha"} (maxAllowed=0s): pending for 58s, firing for 28s:

Jun 15 03:44:53.306 - 28s   W alert/KubePodNotReady ns/openshift-multus pod/network-metrics-daemon-69qsc ALERTS{alertname="KubePodNotReady", alertstate="firing", namespace="openshift-multus", pod="network-metrics-daemon-69qsc", prometheus="openshift-monitoring/k8s", severity="warning"}}

and [bz-Networking][invariant] alert/KubePodNotReady should not be at or above info in ns/openshift-network-diagnostics failed:

{  KubePodNotReady was at or above info for at least 28s on platformidentification.JobType{Release:"4.14", FromRelease:"", Platform:"azure", Architecture:"amd64", Network:"ovn", Topology:"ha"} (maxAllowed=0s): pending for 0s, firing for 28s:

Jun 15 03:44:53.306 - 28s   W alert/KubePodNotReady ns/openshift-network-diagnostics pod/network-check-target-wh2cf ALERTS{alertname="KubePodNotReady", alertstate="firing", namespace="openshift-network-diagnostics", pod="network-check-target-wh2cf", prometheus="openshift-monitoring/k8s", severity="warning"}}

and [bz-Unknown][invariant] alert/KubePodNotReady should not be at or above info in all the other namespaces failed:

{  KubePodNotReady was at or above info for at least 28s on platformidentification.JobType{Release:"4.14", FromRelease:"", Platform:"azure", Architecture:"amd64", Network:"ovn", Topology:"ha"} (maxAllowed=0s): pending for 58s, firing for 28s:

Jun 15 03:44:53.306 - 28s   W alert/KubePodNotReady ns/openshift-e2e-loki pod/loki-promtail-lg6x7 ALERTS{alertname="KubePodNotReady", alertstate="firing", namespace="openshift-e2e-loki", pod="loki-promtail-lg6x7", prometheus="openshift-monitoring/k8s", severity="warning"}}

/test e2e-azure-ovn

e2e-aws-ovn-serial failed because [sig-arch] events should not repeat pathologically failed:

{  2 events happened too frequently

event happened 21 times, something is wrong: node/ip-10-0-96-146.us-west-1.compute.internal hmsg/606919564e - pathological/true reason/ErrorUpdatingResource roles/worker [k8s.ovn.org/node-chassis-id annotation not found for node ip-10-0-96-146.us-west-1.compute.internal, macAddress annotation not found for node "ip-10-0-96-146.us-west-1.compute.internal" , k8s.ovn.org/l3-gateway-config annotation not found for node "ip-10-0-96-146.us-west-1.compute.internal"] From: 00:43:44Z To: 00:43:45Z result=reject 
event happened 22 times, something is wrong: node/ip-10-0-96-146.us-west-1.compute.internal hmsg/606919564e - pathological/true reason/ErrorUpdatingResource roles/worker [k8s.ovn.org/node-chassis-id annotation not found for node ip-10-0-96-146.us-west-1.compute.internal, macAddress annotation not found for node "ip-10-0-96-146.us-west-1.compute.internal" , k8s.ovn.org/l3-gateway-config annotation not found for node "ip-10-0-96-146.us-west-1.compute.internal"] From: 00:43:46Z To: 00:43:47Z result=reject }

This is a known issue: OCPBUGS-10841.
/test e2e-aws-ovn-serial

/hold cancel

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 5, 2023
@Miciah
Copy link
Contributor Author

Miciah commented Aug 5, 2023

/test all
since #934 merged.

@openshift-ci-robot
Copy link
Contributor

/retest-required

Remaining retests: 0 against base HEAD 2be8f80 and 2 for PR HEAD 58f2a10 in total

@Miciah
Copy link
Contributor Author

Miciah commented Aug 6, 2023

e2e-hypershift failed because TestUpgradeControlPlane/EnsureNetworkPolicies/EnsureComponentsHaveNeedManagementKASAccessLabel failed:

{Failed  === RUN   TestUpgradeControlPlane/EnsureNetworkPolicies/EnsureComponentsHaveNeedManagementKASAccessLabel
    util.go:799: 
        Unexpected error:
            <*errors.errorString | 0xc0009a44a0>: 
            not all expected components have NeedManagementKASAccessLabel:   []string(Inverse(Sort, []string{
              	... // 10 identical elements
              	"ignition-server",
              	"machine-approver",
            + 	"olm-collect-profiles-28188048-6qtxc",
              	"private-router",
              }))
            
            {
                s: "not all expected components have NeedManagementKASAccessLabel:   []string(Inverse(Sort, []string{\n  \t... // 10 identical elements\n  \t\"ignition-server\",\n  \t\"machine-approver\",\n+ \t\"olm-collect-profiles-28188048-6qtxc\",\n  \t\"private-router\",\n  }))\n",
            }
        occurred
        --- FAIL: TestUpgradeControlPlane/EnsureNetworkPolicies/EnsureComponentsHaveNeedManagementKASAccessLabel (0.04s)
}

and because TestUpgradeControlPlane/EnsureNetworkPolicies failed:

{Failed  === RUN   TestUpgradeControlPlane/EnsureNetworkPolicies
    --- FAIL: TestUpgradeControlPlane/EnsureNetworkPolicies (2.72s)
}

Cc: @enxebre.
/test e2e-hypershift

@Miciah
Copy link
Contributor Author

Miciah commented Aug 6, 2023

/test e2e-hypershift

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Aug 6, 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.

@openshift-merge-robot openshift-merge-robot merged commit dbbf314 into openshift:master Aug 6, 2023
@openshift-ci-robot
Copy link
Contributor

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

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

Details

In response to this:

Set spec.template.spec.containers[*].ports[*].hostPort on a router deployment if the deployment sets spec.template.spec.hostNetwork to true.

Before this change, the operator left the hostPort field unspecified. As a result, the API set a default value for hostPort. The operator detected this as an external update and attempted to revert it. The change in this PR prevents the operator from making these spurious updates in response to API defaulting.

  • pkg/operator/controller/ingress/deployment.go (desiredRouterDeployment): Set each container port's HostPort to the port's ContainerPort value when HostNetwork is enabled for the deployment.
  • pkg/operator/controller/ingress/deployment_test.go (checkContainerPort): Verify that HostPort is set to ContainerPort if HostNetwork is enabled.

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 May 23, 2024

/cherry-pick release-4.13 release-4.12

@openshift-cherrypick-robot

@candita: new pull request created: #1062

Details

In response to this:

/cherry-pick release-4.13 release-4.12

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-sigs/prow 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. jira/severity-low Referenced Jira bug's severity is low 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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants