OPNET-466: PlatformLoadBalancer becomes GA for On-Prem

[mkowalski]

This PR promotes the PlatformLoadBalancer to GA for bare-metal and vSphere platforms. The feature has been in TP for long enough so that we are ready to promote it.

[openshift-ci-robot]

@mkowalski: This pull request references OPNET-466 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.16.0" version, but no target version was set.

In response to this:

This PR promotes the PlatformLoadBalancer to GA for bare-metal and vSphere platforms. The feature has been in TP for long enough so that we are ready to promote it.

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 openshift-eng/jira-lifecycle-plugin repository.

[openshift-ci]

Hello @mkowalski! Some important instructions when contributing to openshift/api:
API design plays an important part in the user experience of OpenShift and as such API PRs are subject to a high level of scrutiny to ensure they follow our best practices. If you haven't already done so, please review the OpenShift API Conventions and ensure that your proposed changes are compliant. Following these conventions will help expedite the api review process for your PR.

[mkowalski]

/cc @cybertron
/cc @JoelSpeed

[mkowalski]

/retest-required

[JoelSpeed]

Code changes look good, do we have any E2E or QE validation that can show that the feature is working as expected? Can we get an ack from the relevant lead/staff engineer for the feature that they're also happy to promote this?

[mkowalski]

@zhaozhanqi will be QE-ing this as part of OPNET-305. We do not have pre-merge CI (and probably won't have because of the complex underlying infrastructure) so for this one we rely purely on the QE (which may have a smart automation infra for this).

Maybe @sadasu can shed more light as On-Prem team only bumps from TP to GA here, but we didn't do the initial implementation.

(the longer story is that this feature is already GA for OpenStack for quite some time, then it was developed as TP for Bare Metal and vSphere and as now we got actual usage, we promote it for those 2 platforms to GA; OpenStack platform is the reference here as it's been GA-ed already)

[sadasu]

Maybe @sadasu can shed more light as On-Prem team only bumps from TP to GA here, but we didn't do the initial implementation.

@dtantsur Could PTAL at #1757 (comment) and provide the BareMetal perspective?

[sadasu]

Maybe @sadasu can shed more light as On-Prem team only bumps from TP to GA here, but we didn't do the initial implementation.

(the longer story is that this feature is already GA for OpenStack for quite some time, then it was developed as TP for Bare Metal and vSphere and as now we got actual usage, we promote it for those 2 platforms to GA; OpenStack platform is the reference here as it's been GA-ed already)

@rvanderp3 Could you please provide the vSphere perspective?

[dtantsur]

We don't have any opinion on load balancers, I'll trust @mkowalski's judgement here.

[mkowalski]

I resorted to git blame config/v1/types_infrastructure.go and I can see that it was @EmilienM who initially implemented PlatformLoadBalancerType for all platforms as TechPreview (via a660f60 / #1391). It has been hanging as TP for a year already, can you give your OK/NOK for promoting it?

Also cc @eurijon as you gave qe-approved to the TechPreview implementation.

[EmilienM]

The API hasn't changed for a while and I think it's stable enough to be GA. We have pretty good testing coverage on the OpenStack platform.

/lgtm

[EmilienM]

For CI coverage: https://prow.ci.openshift.org/view/gs/test-platform-results/logs/periodic-ci-shiftstack-shiftstack-ci-main-periodic-4.16-e2e-openstack-externallb/1760445181607284736

Yes the job is red, failing on pathological event should not see excessive Back-off restarting failed containers but other than that it's fine.

[cybertron]

/lgtm

from the on-prem networking team lead. Most of the issues we've run into while testing this are related to the fact that it is tech preview in some places and GA in others. We have not found any issues specific to the VSphere or Baremetal platforms that would make us think it is not ready for GA there as well.

[JoelSpeed]

Can we get a QE ack and then I think we have all the relevant acks and can get this merged

[EmilienM]

Ping @eurijon please

[eurijon]

Hello, I'm not sure how related is this to Openstack... we tested external LB functionality in 4.13 as TP and as GA starting in 4.14, by adding

    loadBalancer:
      type: UserManaged

to the install-config with platform: openstack

[mkowalski]

I'm not sure how related is this to Openstack

It's now the same feature but for people with platform: vsphere and so on

[mkowalski]

@JoelSpeed rebased, let me know if all is okay as there are some new intermediary files that appeared in the meantime

[JoelSpeed]

Rather than removing the gates, please add Default to the feature gate,

api/config/v1/feature_gates.go

Line 533 in 07dc95a

enableIn(TechPreviewNoUpgrade).

Once this has been promoted and accepted, in the next release, you can remove the gate and the annotations on the fields themselves

[JoelSpeed]

/lgtm

[openshift-ci-robot]

/retest-required

Remaining retests: 0 against base HEAD ad26456 and 2 for PR HEAD 2cdac33 in total

[openshift-ci-robot]

/retest-required

Remaining retests: 0 against base HEAD c691737 and 1 for PR HEAD 2cdac33 in total

[openshift-ci-robot]

/retest-required

Remaining retests: 0 against base HEAD c542849 and 0 for PR HEAD 2cdac33 in total

[openshift-ci-robot]

/hold

Revision 2cdac33 was retested 3 times: holding

[mkowalski]

Hey @JoelSpeed, can you relabel please? I did not run make update-codegen-crds correctly before

[mkowalski]

/hold cancel

[openshift-ci]

@mkowalski: The following test 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-gcp	e8a9eab	link	false	/test e2e-gcp

Full PR test history. Your PR dashboard.

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.

[JoelSpeed]

/lgtm

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cybertron, EmilienM, JoelSpeed, mkowalski, zhaozhanqi

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:

• OWNERS [JoelSpeed]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment