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

OCPBUGS-2334: Added nil check for service object on load balancer scope change #839

Merged
merged 1 commit into from
Oct 15, 2022

Conversation

gcs278
Copy link
Contributor

@gcs278 gcs278 commented Oct 14, 2022

Added nil check for service object which without causes the ingress operator to panic when using ingress.operator.openshift.io/auto-delete-load-balancer annotation and changing scope of loadbalancer from external to internal.

  • pkg/operator/controller/ingress/status.go: Add nil check for computeAllowedSourceRanges
  • pkg/operator/controller/ingress/status_test.go: Add unit test for computeAllowedSourceRanges

@gcs278
Copy link
Contributor Author

gcs278 commented Oct 14, 2022

/retest

@thejasn
Copy link
Contributor

thejasn commented Oct 14, 2022

/retest

Unrelated failure

Trying to pull registry.build01.ci.openshift.org/ci/managed-clonerefs@sha256:b474342ba1c6eea0d6428b32f6bcd4ceafed02a9e530881dc4fea2138f99c8a2...
Getting image source signatures
error: error creating buildah builder: reading signatures: downloading signatures for sha256:b474342ba1c6eea0d6428b32f6bcd4ceafed02a9e530881dc4fea2138f99c8a2 in registry.build01.ci.openshift.org/ci/managed-clonerefs: received unexpected HTTP status: 504 Gateway Time-out 

@suleymanakbas91
Copy link
Contributor

/retest

1 similar comment
@suleymanakbas91
Copy link
Contributor

/retest

@suleymanakbas91
Copy link
Contributor

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Oct 14, 2022
@suleymanakbas91
Copy link
Contributor

/retest

Pod pending timeout again..

@suleymanakbas91
Copy link
Contributor

/retest

Pod pending timeout again..

@suleymanakbas91
Copy link
Contributor

/retest

Pod pending timeout again..

@gcs278
Copy link
Contributor Author

gcs278 commented Oct 14, 2022

/retest

@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Oct 14, 2022
@suleymanakbas91
Copy link
Contributor

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Oct 14, 2022
@gcs278
Copy link
Contributor Author

gcs278 commented Oct 14, 2022

=== CONT TestAll/parallel/TestUnmanagedDNSToManagedDNSInternalIngressController util_test.go:586: failed to verify connectivity with workload with address: internal-a60e2adcd7286482dab381e0d221f4ad-1811690520.us-east-2.elb.amazonaws.com using internal curl client: timed out waiting for the condition

No TestScopeChange errors!
/retest

@Miciah
Copy link
Contributor

Miciah commented Oct 14, 2022

Could you add a reference/link to the bug in the commit message and put reference to the bug in the PR title?

@gcs278
Copy link
Contributor Author

gcs278 commented Oct 14, 2022

/retitle OCPBUGS-2334: Added nil check for service object on load balancer scope change

@openshift-ci openshift-ci bot changed the title Fixed nil check for service object on load balancer scope change OCPBUGS-2334: Added nil check for service object on load balancer scope change Oct 14, 2022
@openshift-ci-robot
Copy link
Contributor

@gcs278: An error was encountered querying GitHub for users with public email (mjoseph@redhat.com) for bug OCPBUGS-2334 on the Jira server at https://issues.redhat.com/. No known errors were detected, please see the full error message for details.

Full error message. Post "http://ghproxy/graphql": dial tcp 172.30.229.2:80: i/o timeout

Please contact an administrator to resolve this issue, then request a bug refresh with /jira refresh.

In response to this:

Added nil check for service object which without causes the ingress operator to panic when using ingress.operator.openshift.io/auto-delete-load-balancer annotation and changing scope of loadbalancer from external to internal.

  • pkg/operator/controller/ingress/status.go: Add nil check for computeAllowedSourceRanges
  • pkg/operator/controller/ingress/status_test.go: Add unit test for computeAllowedSourceRanges

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.

@gcs278
Copy link
Contributor Author

gcs278 commented Oct 14, 2022

No TestScopeChange issues on round # 2!
Just --- FAIL: TestAll/parallel/TestReloadInterval (147.61s)

…hen using

ingress.operator.openshift.io/auto-delete-load-balancer annotation and
changing scope of loadbalancer from external to internal.

Add unit tests for computeAllowedSourceRanges.
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Oct 14, 2022
@gcs278
Copy link
Contributor Author

gcs278 commented Oct 14, 2022

Could you add a reference/link to the bug in the commit message and put reference to the bug in the PR title?

@Miciah done. I'm using OCPBUGS-2334, let me know if that isn't the bug you want, but I think it fixes it based on testing and the runs we've had so far.

@gcs278
Copy link
Contributor Author

gcs278 commented Oct 14, 2022

/jira refresh

@openshift-ci-robot openshift-ci-robot added the jira/severity-critical Referenced Jira bug's severity is critical for the branch this PR is targeting. label Oct 14, 2022
@openshift-ci-robot openshift-ci-robot added 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 Oct 14, 2022
@openshift-ci-robot
Copy link
Contributor

@gcs278: This pull request references Jira Issue OCPBUGS-2334, which is valid.

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

Requesting review from QA contact:
/cc @melvinjoseph86

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

In response to this:

/jira 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.

@Miciah
Copy link
Contributor

Miciah commented Oct 14, 2022

Thanks!
/lgtm
/approve

@Miciah
Copy link
Contributor

Miciah commented Oct 14, 2022

/jira refresh

@openshift-ci-robot
Copy link
Contributor

@Miciah: This pull request references Jira Issue OCPBUGS-2334, which is valid.

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

Requesting review from QA contact:
/cc @melvinjoseph86

In response to this:

/jira 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.

@Miciah
Copy link
Contributor

Miciah commented Oct 14, 2022

https://issues.redhat.com/browse/OCPBUGS-2334 still points to #840. I'll update it manually.

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Oct 14, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 14, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Miciah

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 Oct 14, 2022
@Miciah
Copy link
Contributor

Miciah commented Oct 14, 2022

Hm, I'll have to ask someone to help me update Jira.

@openshift-ci-robot
Copy link
Contributor

/retest-required

Remaining retests: 0 against base HEAD 24e1863 and 2 for PR HEAD 56b9873 in total

@gcs278
Copy link
Contributor Author

gcs278 commented Oct 14, 2022

/retest
Disruption issues, not related.

@Miciah
Copy link
Contributor

Miciah commented Oct 14, 2022

e2e-aws-operator failed because one of the control-plane nodes was unschedulable, causing numerous components to fail to roll out (dns, image-registry, kube-apiserver, kube-controller-manager, kube-scheduler, network, storage).

/test e2e-aws-operator

@Miciah
Copy link
Contributor

Miciah commented Oct 15, 2022

Most of the failures of the e2e-aws-operator job on this PR have been related to build failures, install failures, or failures in unrelated components. In the cases of two failures (1581056173417697280, 1580986810715082752), the e2e-aws-operator job ran through the operator test suite and failed on tests that are known to be flaky: TestReloadInterval (which #842 will deflake) and TestUnmanagedDNSToManagedDNSInternalIngressController. Based on these results and the fact that this PR fixes a consistent cause of e2e-aws-operator test failures, I am overriding the e2e-aws-operator job for this PR.
/override ci/prow/e2e-aws-operator

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 15, 2022

@Miciah: Overrode contexts on behalf of Miciah: ci/prow/e2e-aws-operator

In response to this:

Most of the failures of the e2e-aws-operator job on this PR have been related to build failures, install failures, or failures in unrelated components. In the cases of two failures (1581056173417697280, 1580986810715082752), the e2e-aws-operator job ran through the operator test suite and failed on tests that are known to be flaky: TestReloadInterval (which #842 will deflake) and TestUnmanagedDNSToManagedDNSInternalIngressController. Based on these results and the fact that this PR fixes a consistent cause of e2e-aws-operator test failures, I am overriding the e2e-aws-operator job for this PR.
/override ci/prow/e2e-aws-operator

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

Miciah commented Oct 15, 2022

/override ci/prow/e2e-gcp-ovn-serial

@Miciah
Copy link
Contributor

Miciah commented Oct 15, 2022

/refresh

@openshift-merge-robot openshift-merge-robot merged commit 6f63bd3 into openshift:master Oct 15, 2022
@openshift-ci-robot
Copy link
Contributor

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

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

In response to this:

Added nil check for service object which without causes the ingress operator to panic when using ingress.operator.openshift.io/auto-delete-load-balancer annotation and changing scope of loadbalancer from external to internal.

  • pkg/operator/controller/ingress/status.go: Add nil check for computeAllowedSourceRanges
  • pkg/operator/controller/ingress/status_test.go: Add unit test for computeAllowedSourceRanges

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/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. jira/severity-critical Referenced Jira bug's severity is critical 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. 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