🌱 CAPD Machines now reports `status.addresses` #3250

andrewsykim · 2020-06-24T19:36:34Z

Signed-off-by: Andrew Sy Kim [email protected]

What this PR does / why we need it:
Adds machine addresses to DockerMachine

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #

andrewsykim · 2020-06-24T20:31:10Z

Some initial testing:

$ kubectl get dockermachine workload-cls-worker-0 -o yaml
apiVersion: infrastructure.cluster.x-k8s.io/v1alpha3
kind: DockerMachine
metadata:
  annotations:
    kubectl.kubernetes.io/last-applied-configuration: |
      {"apiVersion":"infrastructure.cluster.x-k8s.io/v1alpha3","kind":"DockerMachine","metadata":{"annotations":{},"name":"workload-cls-worker-0","namespace":"default"}}
  creationTimestamp: "2020-06-24T20:02:41Z"
  finalizers:
  - dockermachine.infrastructure.cluster.x-k8s.io
  generation: 2
  labels:
    cluster.x-k8s.io/cluster-name: workload-cls
  name: workload-cls-worker-0
  namespace: default
  ownerReferences:
  - apiVersion: cluster.x-k8s.io/v1alpha3
    blockOwnerDeletion: true
    controller: true
    kind: Machine
    name: workload-cls-worker-0
    uid: 6c0004a7-bef0-4e51-a3a4-7ade21258806
  resourceVersion: "2642"
  selfLink: /apis/infrastructure.cluster.x-k8s.io/v1alpha3/namespaces/default/dockermachines/workload-cls-worker-0
  uid: 6f15ac8e-5e42-4b3b-ae76-40d72575168c
spec:
  bootstrapped: true
  providerID: docker:////workload-cls-workload-cls-worker-0
status:
  addresses:
  - address: workload-cls-workload-cls-worker-0
    type: Hostname
  - address: 172.17.0.9
    type: InternalIP
  - address: 172.17.0.9
    type: ExternalIP
  ready: true

$ $ kubectl get machine workload-cls-worker-0 -o yaml
apiVersion: cluster.x-k8s.io/v1alpha3
kind: Machine
metadata:
  annotations:
    kubectl.kubernetes.io/last-applied-configuration: |
      {"apiVersion":"cluster.x-k8s.io/v1alpha3","kind":"Machine","metadata":{"annotations":{},"labels":{"cluster.x-k8s.io/cluster-name":"workload-cls"},"name":"workload-cls-worker-0","namespace":"default"},"spec":{"bootstrap":{"configRef":{"apiVersion":"bootstrap.cluster.x-k8s.io/v1alpha3","kind":"KubeadmConfig","name":"workload-cls-worker-0-config","namespace":"default"}},"clusterName":"workload-cls","infrastructureRef":{"apiVersion":"infrastructure.cluster.x-k8s.io/v1alpha3","kind":"DockerMachine","name":"workload-cls-worker-0","namespace":"default"},"version":"v1.17.2"}}
  creationTimestamp: "2020-06-24T20:02:41Z"
  finalizers:
  - machine.cluster.x-k8s.io
  generation: 3
  labels:
    cluster.x-k8s.io/cluster-name: workload-cls
  name: workload-cls-worker-0
  namespace: default
  ownerReferences:
  - apiVersion: cluster.x-k8s.io/v1alpha3
    kind: Cluster
    name: workload-cls
    uid: ec36fa04-6db0-403e-be5a-2c2449bc079b
  resourceVersion: "2646"
  selfLink: /apis/cluster.x-k8s.io/v1alpha3/namespaces/default/machines/workload-cls-worker-0
  uid: 6c0004a7-bef0-4e51-a3a4-7ade21258806
spec:
  bootstrap:
    configRef:
      apiVersion: bootstrap.cluster.x-k8s.io/v1alpha3
      kind: KubeadmConfig
      name: workload-cls-worker-0-config
      namespace: default
    dataSecretName: workload-cls-worker-0-config
  clusterName: workload-cls
  infrastructureRef:
    apiVersion: infrastructure.cluster.x-k8s.io/v1alpha3
    kind: DockerMachine
    name: workload-cls-worker-0
    namespace: default
  providerID: docker:////workload-cls-workload-cls-worker-0
  version: v1.17.2
status:
  addresses:
  - address: workload-cls-workload-cls-worker-0
    type: Hostname
  - address: 172.17.0.9
    type: InternalIP
  - address: 172.17.0.9
    type: ExternalIP
  bootstrapReady: true
  infrastructureReady: true
  lastUpdated: "2020-06-24T20:04:57Z"
  nodeRef:
    name: workload-cls-workload-cls-worker-0
    uid: 23ed00f0-a61c-439c-8d19-53e77941185f
  phase: Running

andrewsykim · 2020-06-24T21:37:34Z

/retest

andrewsykim · 2020-06-24T22:10:27Z

/assign @ncdc

fabriziopandini · 2020-06-25T09:21:57Z

test/infrastructure/docker/controllers/dockermachine_controller.go

@@ -277,6 +277,28 @@ func (r *DockerMachineReconciler) reconcileNormal(ctx context.Context, machine *
 	dockerMachine.Status.Ready = true
 	conditions.MarkTrue(dockerMachine, infrav1.ContainerProvisionedCondition)

+	// set address in machine status


the code for reconciling network addresses (L280-301) should go before the block of code that reconcile ProviderID (L267-268), otherwise, in case of errors the network won't be set due to the test at L172

fabriziopandini · 2020-06-25T09:22:21Z

test/infrastructure/docker/controllers/dockermachine_controller.go

+	// set address in machine status
+	machineAddress, err := externalMachine.Address()
+	if err != nil {
+		r.Log.Error(err, "failed to patch the Kubernetes node with the machine address")


Suggested change

r.Log.Error(err, "failed to patch the Kubernetes node with the machine address")

r.Log.Error(err, "failed to get the machine address")

ncdc · 2020-06-25T14:22:17Z

test/infrastructure/docker/docker/machine.go

@@ -104,6 +104,15 @@ func (m *Machine) ProviderID() string {
 	return fmt.Sprintf("docker:////%s", m.ContainerName())
 }

+func (m *Machine) Address() (string, error) {
+	ipv4, _, err := m.container.IP(context.TODO())


We do have a context in the calling function that you could pass through - want to do that instead of using TODO here?

yeah good catch

Signed-off-by: Andrew Sy Kim <[email protected]>

ncdc · 2020-06-25T14:29:48Z

/approve
/assign @fabriziopandini

k8s-ci-robot · 2020-06-25T14:29:57Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ncdc

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:

~~test/infrastructure/docker/OWNERS~~ [ncdc]

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

fabriziopandini · 2020-06-25T15:10:15Z

/lgtm

fabriziopandini · 2020-06-25T15:42:51Z

/retest

vincepri

/milestone v0.3.7
/retitle 🌱 CAPD Machines now report status.addresses

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jun 24, 2020

k8s-ci-robot requested review from detiber and vincepri June 24, 2020 19:37

andrewsykim force-pushed the capd-machine-address branch from 121e31e to f9dd724 Compare June 24, 2020 19:40

andrewsykim force-pushed the capd-machine-address branch from f9dd724 to 5cbe5d3 Compare June 24, 2020 20:39

k8s-ci-robot assigned ncdc Jun 24, 2020

fabriziopandini reviewed Jun 25, 2020

View reviewed changes

ncdc reviewed Jun 25, 2020

View reviewed changes

andrewsykim force-pushed the capd-machine-address branch from 5cbe5d3 to 4250835 Compare June 25, 2020 14:23

capd: set machine addresses

bebff1b

Signed-off-by: Andrew Sy Kim <[email protected]>

andrewsykim force-pushed the capd-machine-address branch from 4250835 to bebff1b Compare June 25, 2020 14:26

k8s-ci-robot assigned fabriziopandini Jun 25, 2020

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 25, 2020

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 25, 2020

vincepri approved these changes Jun 25, 2020

View reviewed changes

k8s-ci-robot changed the title ~~capd: set machine addresses~~ 🌱 CAPD Machines now report status.addresses Jun 25, 2020

k8s-ci-robot added this to the v0.3.7 milestone Jun 25, 2020

vincepri changed the title ~~🌱 CAPD Machines now report status.addresses~~ 🌱 CAPD Machines now reports status.addresses Jun 25, 2020

k8s-ci-robot merged commit fd73b18 into kubernetes-sigs:master Jun 25, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🌱 CAPD Machines now reports `status.addresses` #3250

🌱 CAPD Machines now reports `status.addresses` #3250

andrewsykim commented Jun 24, 2020

andrewsykim commented Jun 24, 2020

andrewsykim commented Jun 24, 2020

andrewsykim commented Jun 24, 2020

fabriziopandini Jun 25, 2020 •

edited

Loading

andrewsykim Jun 25, 2020

fabriziopandini Jun 25, 2020

andrewsykim Jun 25, 2020

ncdc Jun 25, 2020

andrewsykim Jun 25, 2020

andrewsykim Jun 25, 2020

ncdc commented Jun 25, 2020

k8s-ci-robot commented Jun 25, 2020

fabriziopandini commented Jun 25, 2020

fabriziopandini commented Jun 25, 2020

vincepri left a comment

	r.Log.Error(err, "failed to patch the Kubernetes node with the machine address")
	r.Log.Error(err, "failed to get the machine address")

🌱 CAPD Machines now reports status.addresses #3250

🌱 CAPD Machines now reports status.addresses #3250

Conversation

andrewsykim commented Jun 24, 2020

andrewsykim commented Jun 24, 2020

andrewsykim commented Jun 24, 2020

andrewsykim commented Jun 24, 2020

fabriziopandini Jun 25, 2020 • edited Loading

Choose a reason for hiding this comment

andrewsykim Jun 25, 2020

Choose a reason for hiding this comment

fabriziopandini Jun 25, 2020

Choose a reason for hiding this comment

andrewsykim Jun 25, 2020

Choose a reason for hiding this comment

ncdc Jun 25, 2020

Choose a reason for hiding this comment

andrewsykim Jun 25, 2020

Choose a reason for hiding this comment

andrewsykim Jun 25, 2020

Choose a reason for hiding this comment

ncdc commented Jun 25, 2020

k8s-ci-robot commented Jun 25, 2020

fabriziopandini commented Jun 25, 2020

fabriziopandini commented Jun 25, 2020

vincepri left a comment

Choose a reason for hiding this comment

🌱 CAPD Machines now reports `status.addresses` #3250

🌱 CAPD Machines now reports `status.addresses` #3250

fabriziopandini Jun 25, 2020 •

edited

Loading