kubernetes-sigs / mcs-api

This repository hosts the Multi-Cluster Service APIs. Providers can import packages in this repo to ensure their multi-cluster service controller implementations will be compatible with MCS data planes.
Apache License 2.0
199 stars 39 forks source link

Assign predefined clustersetIP to service status #1

Closed andrewsykim closed 3 years ago

andrewsykim commented 3 years ago

This PR updates the current demo to to set Service.Status.LoadBalancer if clusterset IPs is predefined in ServiceImport.

This allows the implementation to rely on clusterIP for ServiceImport.Spec.IPs or pre-allocate a well known IP for the clustersetIP.

Signed-off-by: Andrew Sy Kim kim.andrewsy@gmail.com

andrewsykim commented 3 years ago

/assign @JeremyOT

andrewsykim commented 3 years ago

Some manual validation running the demo script:

$ kubectl --kubeconfig scripts/c1.kubeconfig -n demo get serviceimports
NAME    TYPE           IP          AGE
serve   ClusterSetIP   [1.2.3.4]   13m
$
$ kubectl --kubeconfig scripts/c1.kubeconfig -n demo get svc
NAME                 TYPE        CLUSTER-IP     EXTERNAL-IP   PORT(S)   AGE
derived-ovtkn0laet   ClusterIP   10.11.78.176   <none>        80/TCP    2m36s
serve                ClusterIP   10.11.174.93   <none>        80/TCP    2m49s
$ 
$ kubectl --kubeconfig scripts/c1.kubeconfig -n demo get svc derived-ovtkn0laet -o yaml
apiVersion: v1
kind: Service
metadata:
  creationTimestamp: "2020-08-11T15:22:26Z"
  managedFields:
  - apiVersion: v1
    fieldsType: FieldsV1
    fieldsV1:
      f:metadata:
        f:ownerReferences:
          .: {}
          k:{"uid":"edd5dedb-5ee7-4b57-8c91-9654bff70449"}:
            .: {}
            f:apiVersion: {}
            f:kind: {}
            f:name: {}
            f:uid: {}
      f:spec:
        f:ports:
          .: {}
          k:{"port":80,"protocol":"TCP"}:
            .: {}
            f:port: {}
            f:protocol: {}
            f:targetPort: {}
        f:sessionAffinity: {}
        f:type: {}
      f:status:
        f:loadBalancer:
          f:ingress: {}
    manager: controller
    operation: Update
    time: "2020-08-11T15:22:26Z"
  name: derived-ovtkn0laet
  namespace: demo
  ownerReferences:
  - apiVersion: multicluster.x-k8s.io/v1alpha1
    kind: ServiceImport
    name: serve
    uid: edd5dedb-5ee7-4b57-8c91-9654bff70449
  resourceVersion: "688"
  selfLink: /api/v1/namespaces/demo/services/derived-ovtkn0laet
  uid: a502033b-aab5-491d-8b7f-25a5ff32b3f6
spec:
  clusterIP: 10.11.78.176
  ports:
  - port: 80
    protocol: TCP
    targetPort: 80
  sessionAffinity: None
  type: ClusterIP
status:
  loadBalancer:
    ingress:
    - ip: 1.2.3.4
andrewsykim commented 3 years ago

Based on some conversations with @JeremyOT , I've updated the PR so that we add IPs to service status if a clustersetIP is predefined. Otherwise we continue to put clusterIP into ServiceImport.Spec.IPs

$ kubectl --kubeconfig ./scripts/c1.kubeconfig get serviceimport -A
NAMESPACE   NAME             TYPE           IP                AGE
demo        serve            ClusterSetIP   [10.11.104.125]   2m6s
demo        serve-with-vip   ClusterSetIP   [1.2.3.4]         2m5s
JeremyOT commented 3 years ago

Thanks for this!

JeremyOT commented 3 years ago

/lgtm /approve

k8s-ci-robot commented 3 years ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: andrewsykim, JeremyOT

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](https://github.com/kubernetes-sigs/mcs-api/blob/master/OWNERS)~~ [JeremyOT] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment