Deprecate Service.Spec.LoadBalancerIP

[uablrek]

What type of PR is this?

/kind documentation
/kind deprecation
/sig network

What this PR does / why we need it:

Service.Spec.LoadBalancerIP can't be used for dual-stack services. A plural LoadBalancerIPs was proposed in PR1992. The discussions around that PR revealed that the Service.Spec.LoadBalancerIP field was under-specified and and its meaning varies across implementations. So the final proposal was to deprecate Service.Spec.LoadBalancerIP rather than introducing a plural LoadBalancerIPs.

Which issue(s) this PR fixes:

None

Special notes for your reviewer:

Please read the comments in PR1992.

Does this PR introduce a user-facing change?

Deprecate Service.Spec.LoadBalancerIP. This field was under-specified and its meaning varies across implementations.  As of Kubernetes v1.24, users are encouraged to use implementation-specific annotations when available.  This field may be removed in a future API version.

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

[k8s-ci-robot]

@uablrek: This issue is currently awaiting triage.

If a SIG or subproject determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

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.

[uablrek]

/assign @thockin

[k8s-triage-robot]

This PR may require API review.

If so, when the changes are ready, complete the pre-review checklist and request an API review.

Status of requested reviews is tracked in the API Review project.

[aojea]

@liggitt do we need to add code for adding the warning headers? any pointer?

[thockin]

/lgtm
/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: thockin, uablrek

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:

• api/OWNERS [thockin]
• pkg/apis/OWNERS [thockin]
• staging/src/k8s.io/api/OWNERS [thockin]

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

[k8s-triage-robot]

The Kubernetes project has merge-blocking tests that are currently too flaky to consistently pass.

This bot retests PRs for certain kubernetes repos according to the following rules:

• The PR does have any do-not-merge/* labels
• The PR does not have the needs-ok-to-test label
• The PR is mergeable (does not have a needs-rebase label)
• The PR is approved (has cncf-cla: yes, lgtm, approved labels)
• The PR is failing tests required for merge

You can:

• Review the full test history for this PR
• Prevent this bot from retesting with /lgtm cancel or /hold
• Help make our tests less flaky by following our Flaky Tests Guide

/retest

[aojea]

@thockin @uablrek it seems that this fields is in use by all the cloud providers and the warning about removal is forcing them to move to annotations kubernetes/cloud-provider-gcp#371

I don't know if we can do a breaking change like that of removing it in the future, should we relax the wording ?

[uablrek]

it seems that this fields is in use by all the cloud providers and the warning about removal is forcing them to move to annotations

I think that's the intention. I assume no cloud provider just use the LoadBalancerIP address without other configurations as well, so this really is a cloud provider specific setting and should be an annotation.

[aojea]

But I don't think we can remove it without breaking users and keep compatibility with old clients?

apiVersion: v1
kind: Service
metadata:
  name:test
  namespace: default
spec:
  nonExistingField: test
  ports:
  - port: 80
    targetPort: 80
    protocol: TCP
  selector:
    app: echoserver
  type: ClusterIP

$ kubectl create -f svc2.yaml
Error from server (BadRequest): error when creating "svc2.yaml": Service in version "v1" cannot be handled as a Service: strict decoding error: unknown field "spec.nonExistingField"

@liggitt @thockin is realistic we can remove this field without going through a large process of deprecation?
how do we deal with stored objects that already has the field set?

[liggitt]

Agree we would not drop the field, but marking it deprecated/undefined behavior is fair game.

[aojea]

Just to avoid more confusion on this topic, deprecating API and remove fields on GA APIs has to obey some rules

See API changes

https://kubernetes.io/docs/concepts/overview/kubernetes-api/#api-changes

Elimination of resources or fields requires following the API deprecation policy.

See the rules

https://kubernetes.io/docs/reference/using-api/deprecation-policy/#deprecating-parts-of-the-api

Fields of REST resources
As with whole REST resources, an individual field which was present in API v1 must exist and function until API v1 is removed. Unlike whole resources, the v2 APIs may choose a different representation for the field, as long as it can be round-tripped. For example a v1 field named "magnitude" which was deprecated might be named "deprecatedMagnitude" in API v2. When v1 is eventually removed, the deprecated field can be removed from v2.