Skip to content

OCPBUGS-2436: Fix ingress config with empty lbType on AWS#6489

Closed
Miciah wants to merge 1 commit intoopenshift:masterfrom
Miciah:OCPBUGS-2436-fix-ingress-config-with-empty-lbType-on-AWS
Closed

OCPBUGS-2436: Fix ingress config with empty lbType on AWS#6489
Miciah wants to merge 1 commit intoopenshift:masterfrom
Miciah:OCPBUGS-2436-fix-ingress-config-with-empty-lbType-on-AWS

Conversation

@Miciah
Copy link
Contributor

@Miciah Miciah commented Oct 16, 2022

When generating the ingresses.config.openshift.io/cluster manifest on AWS, don't set spec.loadBalancer unless the install-config lbType is nonempty.

Before this PR, if lbType was empty, the installer generated a manifest with the following:

  loadBalancer:
    platform:
      aws: {}
      type: AWS

Whereas loadBalancer, platform, and aws are optional fields, the type subfield of the aws field is required. As a consequence, the cluster bootstrap would fail with the following error:

    "cluster-ingress-02-config.yml": failed to create ingresses.v1.config.openshift.io/cluster -n : Ingress.config.openshift.io "cluster" is invalid: spec.loadBalancer.platform.aws.type: Required value

This PR ensures that the installer doesn't generate an invalid ingress manifest when lbType is empty.

Follow-up to #6478.

  • pkg/asset/manifests/ingress.go (generateClusterConfig): Don't set spec.loadBalancer unless we have a nonempty lbType value to put in spec.loadBalancer.platform.aws.type.

When generating the ingresses.config.openshift.io/cluster manifest on AWS,
don't set spec.loadBalancer unless the install-config lbType is nonempty.

Before this commit, if lbType was empty, the installer generated a manifest
with the following:

  loadBalancer:
    platform:
      aws: {}
      type: AWS

Whereas loadBalancer, platform, and aws are optional fields, the type
subfield of the aws field is required.  As a consequence, the cluster
bootstrap would fail with the following error:

    "cluster-ingress-02-config.yml": failed to create ingresses.v1.config.openshift.io/cluster -n : Ingress.config.openshift.io "cluster" is invalid: spec.loadBalancer.platform.aws.type: Required value

This commit ensures that the installer doesn't generate an invalid ingress
manifest when lbType is empty.

Follow-up to commit 5d12adc.

This commit fixes bug OCPBUGS-2436.

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

* pkg/asset/manifests/ingress.go (generateClusterConfig): Don't set
spec.loadBalancer unless we have a nonempty lbType value to put in
spec.loadBalancer.platform.aws.type.
@openshift-ci-robot openshift-ci-robot added jira/severity-critical Referenced Jira bug's severity is critical for the branch this PR is targeting. jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. labels Oct 16, 2022
@openshift-ci-robot
Copy link
Contributor

@Miciah: This pull request references Jira Issue OCPBUGS-2436, which is invalid:

  • expected the bug to target the "4.12.0" version, but it targets "4.12.z" instead

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

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

Details

In response to this:

When generating the ingresses.config.openshift.io/cluster manifest on AWS, don't set spec.loadBalancer unless the install-config lbType is nonempty.

Before this PR, if lbType was empty, the installer generated a manifest
with the following:

 loadBalancer:
   platform:
     aws: {}
     type: AWS

Whereas loadBalancer, platform, and aws are optional fields, the type subfield of the aws field is required. As a consequence, the cluster bootstrap would fail with the following error:

   "cluster-ingress-02-config.yml": failed to create ingresses.v1.config.openshift.io/cluster -n : Ingress.config.openshift.io "cluster" is invalid: spec.loadBalancer.platform.aws.type: Required value

This PR ensures that the installer doesn't generate an invalid ingress manifest when lbType is empty.

Follow-up to #6478.

  • pkg/asset/manifests/ingress.go (generateClusterConfig): Don't set spec.loadBalancer unless we have a nonempty lbType value to put in spec.loadBalancer.platform.aws.type.

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 barbacbd and sadasu October 16, 2022 18:39
@openshift-ci-robot
Copy link
Contributor

@Miciah: This pull request references Jira Issue OCPBUGS-2436, which is invalid:

  • expected the bug to target the "4.12.0" version, but it targets "4.12.z" instead

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

Details

In response to this:

When generating the ingresses.config.openshift.io/cluster manifest on AWS, don't set spec.loadBalancer unless the install-config lbType is nonempty.

Before this PR, if lbType was empty, the installer generated a manifest with the following:

 loadBalancer:
   platform:
     aws: {}
     type: AWS

Whereas loadBalancer, platform, and aws are optional fields, the type subfield of the aws field is required. As a consequence, the cluster bootstrap would fail with the following error:

   "cluster-ingress-02-config.yml": failed to create ingresses.v1.config.openshift.io/cluster -n : Ingress.config.openshift.io "cluster" is invalid: spec.loadBalancer.platform.aws.type: Required value

This PR ensures that the installer doesn't generate an invalid ingress manifest when lbType is empty.

Follow-up to #6478.

  • pkg/asset/manifests/ingress.go (generateClusterConfig): Don't set spec.loadBalancer unless we have a nonempty lbType value to put in spec.loadBalancer.platform.aws.type.

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 Author

Miciah commented Oct 16, 2022

/jira refresh

@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 16, 2022
@openshift-ci-robot
Copy link
Contributor

@Miciah: This pull request references Jira Issue OCPBUGS-2436, 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.12.0) matches configured target version for branch (4.12.0)
  • bug is in the state New, which is one of the valid states (NEW, ASSIGNED, POST)

No GitHub users were found matching the public email listed for the QA contact in Jira (gpei@redhat.com), skipping review request.

Details

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.

@openshift-ci-robot openshift-ci-robot removed the jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. label Oct 16, 2022
@miheer
Copy link
Contributor

miheer commented Oct 16, 2022

/lgtm

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

miheer commented Oct 16, 2022

thanks @Miciah !

@patrickdillon
Copy link
Contributor

/approve

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 16, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: patrickdillon

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 Oct 16, 2022
@r4f4
Copy link
Contributor

r4f4 commented Oct 16, 2022

/test e2e-aws-ovn

@openshift-ci-robot
Copy link
Contributor

/retest-required

Remaining retests: 0 against base HEAD 85c8d56 and 2 for PR HEAD b31c4ea in total

@frobware
Copy link

/skip
/retest

@miheer
Copy link
Contributor

miheer commented Oct 17, 2022

/retest-required

1 similar comment
@JoelSpeed
Copy link
Contributor

/retest-required

@stbenjam
Copy link
Member

stbenjam commented Oct 17, 2022

/hold

After the revert lands, can you build this fix on top of unreverting, and then make sure it passes the jobs that are currently broken on the payload? It looks like aws-sdn-serial is the clearest one. You can use /payload-job periodic-ci-openshift-release-master-nightly-4.12-e2e-aws-sdn-serial. 🙇🏻‍♂️

@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 Oct 17, 2022
@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 17, 2022
@openshift-merge-robot
Copy link
Contributor

@Miciah: PR needs rebase.

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.

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 17, 2022

@Miciah: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-openstack b31c4ea link false /test e2e-openstack
ci/prow/okd-scos-e2e-aws-upgrade b31c4ea link false /test okd-scos-e2e-aws-upgrade
ci/prow/e2e-libvirt b31c4ea link false /test e2e-libvirt
ci/prow/e2e-metal-ipi b31c4ea link false /test e2e-metal-ipi
ci/prow/okd-e2e-aws-upgrade b31c4ea link false /test okd-e2e-aws-upgrade
ci/prow/okd-e2e-aws-ovn b31c4ea link false /test okd-e2e-aws-ovn
ci/prow/okd-scos-e2e-aws-ovn b31c4ea link false /test okd-scos-e2e-aws-ovn
ci/prow/e2e-aws-ovn b31c4ea link true /test e2e-aws-ovn

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 Oct 17, 2022

/close
because #6490 has merged and reverts the problematic code. #6491 reverts #6490 and incorporates the bugfix in this PR.

@openshift-ci openshift-ci bot closed this Oct 17, 2022
@openshift-ci-robot
Copy link
Contributor

@Miciah: This pull request references Jira Issue OCPBUGS-2436. The bug has been updated to no longer refer to the pull request using the external bug tracker.

Details

In response to this:

When generating the ingresses.config.openshift.io/cluster manifest on AWS, don't set spec.loadBalancer unless the install-config lbType is nonempty.

Before this PR, if lbType was empty, the installer generated a manifest with the following:

 loadBalancer:
   platform:
     aws: {}
     type: AWS

Whereas loadBalancer, platform, and aws are optional fields, the type subfield of the aws field is required. As a consequence, the cluster bootstrap would fail with the following error:

   "cluster-ingress-02-config.yml": failed to create ingresses.v1.config.openshift.io/cluster -n : Ingress.config.openshift.io "cluster" is invalid: spec.loadBalancer.platform.aws.type: Required value

This PR ensures that the installer doesn't generate an invalid ingress manifest when lbType is empty.

Follow-up to #6478.

  • pkg/asset/manifests/ingress.go (generateClusterConfig): Don't set spec.loadBalancer unless we have a nonempty lbType value to put in spec.loadBalancer.platform.aws.type.

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

openshift-ci bot commented Oct 17, 2022

@Miciah: Closed this PR.

Details

In response to this:

/close
because #6490 has merged and reverts the problematic code. #6491 reverts #6490 and incorporates the bugfix in this PR.

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. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. 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. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants

Comments