From 84f28a53473f1ba540e9b2aa51736022b4b64235 Mon Sep 17 00:00:00 2001 From: Kostiantyn Kalynovskyi Date: Wed, 24 Mar 2021 23:01:35 +0000 Subject: [PATCH] Allow to specify node labels for copying to BMH With this change, vino-controller will take keys from vino.Spec.CopyNodeLabelKeys and copy corresponding labels from k8s node to a BMHs created. Change-Id: Idd83766a064d406c83ee504dd89b1474aa15c49a --- .../bases/airship.airshipit.org_vinoes.yaml | 21 ++++--- config/samples/vino_cr.yaml | 5 ++ docs/api/vino.md | 63 +++++++++---------- pkg/api/v1/vino_types.go | 16 ++--- pkg/api/v1/zz_generated.deepcopy.go | 35 ++++------- pkg/controllers/bmh.go | 32 +++++++--- pkg/controllers/bmh_test.go | 22 ++++++- tools/deployment/test-cr.sh | 12 +++- 8 files changed, 121 insertions(+), 85 deletions(-) diff --git a/config/crd/bases/airship.airshipit.org_vinoes.yaml b/config/crd/bases/airship.airshipit.org_vinoes.yaml index 6908505..b0ffec1 100644 --- a/config/crd/bases/airship.airshipit.org_vinoes.yaml +++ b/config/crd/bases/airship.airshipit.org_vinoes.yaml @@ -111,6 +111,12 @@ spec: type: string type: object type: array + nodeLabelKeysToCopy: + description: NodeLabelKeysToCopy vino controller will get these labels + from k8s nodes and place them on BMHs that correspond to this node + items: + type: string + type: array nodeSelector: description: Define nodelabel parameters properties: @@ -127,6 +133,13 @@ spec: items: description: NodeSet node definitions properties: + bmhLabels: + additionalProperties: + type: string + description: BMHLabels labels will be copied directly to BMHs + that will be created These labels will override keys from k8s + node, that are specified in vino.NodeLabelKeysToCopy + type: object count: type: integer diskDrives: @@ -147,14 +160,6 @@ spec: type: type: string type: object - labels: - description: VMNodeFlavor labels for node to be annotated - properties: - vmFlavor: - additionalProperties: - type: string - type: object - type: object libvirtTemplate: description: NamespacedName to be used to spawn VMs properties: diff --git a/config/samples/vino_cr.yaml b/config/samples/vino_cr.yaml index 400ce91..1766b4f 100644 --- a/config/samples/vino_cr.yaml +++ b/config/samples/vino_cr.yaml @@ -4,6 +4,9 @@ metadata: name: vino-test-cr # labels: ... spec: + nodeLabelKeysToCopy: + - "airshipit.org/server" + - "airshipit.org/rack" nodeSelector: matchLabels: beta.kubernetes.io/os: linux @@ -36,6 +39,8 @@ spec: nodes: - name: "worker" count: 3 + bmhLabels: + airshipit.org/k8s-role: worker networkDataTemplate: name: "test-template" namespace: "default" diff --git a/docs/api/vino.md b/docs/api/vino.md index de7de98..3bc69e5 100644 --- a/docs/api/vino.md +++ b/docs/api/vino.md @@ -739,14 +739,14 @@ int -labels
+bmhLabels
- -VMNodeFlavor - +map[string]string +

BMHLabels labels will be copied directly to BMHs that will be created +These labels will override keys from k8s node, that are specified in vino.NodeLabelKeysToCopy

@@ -843,37 +843,6 @@ string -

VMNodeFlavor -

-

-(Appears on: -NodeSet) -

-

VMNodeFlavor labels for node to be annotated

-
-
- - - - - - - - - - - - - -
FieldDescription
-vmFlavor
- -map[string]string - -		 -
-
-

VMRoutes

@@ -1055,6 +1024,18 @@ BMCCredentials sushy tools will use these credentials as well, to set up authentication

+ + +nodeLabelKeysToCopy
+ +[]string + + + +

NodeLabelKeysToCopy vino controller will get these labels from k8s nodes +and place them on BMHs that correspond to this node

+ + @@ -1181,6 +1162,18 @@ BMCCredentials sushy tools will use these credentials as well, to set up authentication

+ + +nodeLabelKeysToCopy
+ +[]string + + + +

NodeLabelKeysToCopy vino controller will get these labels from k8s nodes +and place them on BMHs that correspond to this node

+ + diff --git a/pkg/api/v1/vino_types.go b/pkg/api/v1/vino_types.go index 175579a..794deff 100644 --- a/pkg/api/v1/vino_types.go +++ b/pkg/api/v1/vino_types.go @@ -52,6 +52,9 @@ type VinoSpec struct { // BMCCredentials contain credentials that will be used to create BMH nodes // sushy tools will use these credentials as well, to set up authentication BMCCredentials BMCCredentials `json:"bmcCredentials"` + // NodeLabelKeysToCopy vino controller will get these labels from k8s nodes + // and place them on BMHs that correspond to this node + NodeLabelKeysToCopy []string `json:"nodeLabelKeysToCopy,omitempty"` } // BMCCredentials contain credentials that will be used to create BMH nodes @@ -95,9 +98,11 @@ type VMRoutes struct { //NodeSet node definitions type NodeSet struct { //Parameter for Node master or worker-standard - Name string `json:"name,omitempty"` - Count int `json:"count,omitempty"` - NodeLabel VMNodeFlavor `json:"labels,omitempty"` + Name string `json:"name,omitempty"` + Count int `json:"count,omitempty"` + // BMHLabels labels will be copied directly to BMHs that will be created + // These labels will override keys from k8s node, that are specified in vino.NodeLabelKeysToCopy + BMHLabels map[string]string `json:"bmhLabels,omitempty"` LibvirtTemplateDefinition NamespacedName `json:"libvirtTemplate,omitempty"` NetworkInterfaces []NetworkInterface `json:"networkInterfaces,omitempty"` DiskDrives *DiskDrivesTemplate `json:"diskDrives,omitempty"` @@ -105,11 +110,6 @@ type NodeSet struct { NetworkDataTemplate NamespacedName `json:"networkDataTemplate,omitempty"` } -// VMNodeFlavor labels for node to be annotated -type VMNodeFlavor struct { - VMFlavor map[string]string `json:"vmFlavor,omitempty"` -} - // NamespacedName to be used to spawn VMs type NamespacedName struct { Name string `json:"name,omitempty"` diff --git a/pkg/api/v1/zz_generated.deepcopy.go b/pkg/api/v1/zz_generated.deepcopy.go index 69adb0b..031a33b 100644 --- a/pkg/api/v1/zz_generated.deepcopy.go +++ b/pkg/api/v1/zz_generated.deepcopy.go @@ -307,7 +307,13 @@ func (in *NodeSelector) DeepCopy() *NodeSelector { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *NodeSet) DeepCopyInto(out *NodeSet) { *out = *in - in.NodeLabel.DeepCopyInto(&out.NodeLabel) + if in.BMHLabels != nil { + in, out := &in.BMHLabels, &out.BMHLabels + *out = make(map[string]string, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } out.LibvirtTemplateDefinition = in.LibvirtTemplateDefinition if in.NetworkInterfaces != nil { in, out := &in.NetworkInterfaces, &out.NetworkInterfaces @@ -349,28 +355,6 @@ func (in *Range) DeepCopy() *Range { return out } -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *VMNodeFlavor) DeepCopyInto(out *VMNodeFlavor) { - *out = *in - if in.VMFlavor != nil { - in, out := &in.VMFlavor, &out.VMFlavor - *out = make(map[string]string, len(*in)) - for key, val := range *in { - (*out)[key] = val - } - } -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new VMNodeFlavor. -func (in *VMNodeFlavor) DeepCopy() *VMNodeFlavor { - if in == nil { - return nil - } - out := new(VMNodeFlavor) - in.DeepCopyInto(out) - return out -} - // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *VMRoutes) DeepCopyInto(out *VMRoutes) { *out = *in @@ -474,6 +458,11 @@ func (in *VinoSpec) DeepCopyInto(out *VinoSpec) { } out.DaemonSetOptions = in.DaemonSetOptions out.BMCCredentials = in.BMCCredentials + if in.NodeLabelKeysToCopy != nil { + in, out := &in.NodeLabelKeysToCopy, &out.NodeLabelKeysToCopy + *out = make([]string, len(*in)) + copy(*out, *in) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new VinoSpec. diff --git a/pkg/controllers/bmh.go b/pkg/controllers/bmh.go index 305f7e6..cdc202f 100644 --- a/pkg/controllers/bmh.go +++ b/pkg/controllers/bmh.go @@ -185,19 +185,22 @@ func (r *VinoReconciler) createBMHperPod(ctx context.Context, vino *vinov1.Vino, return err } - // TODO extend this function to return server/rack labels as well - bmcAddr, err := r.getBMCAddress(ctx, pod, roleSuffix) + bmcAddr, labels, err := r.getBMCAddressAndLabels(ctx, pod, vino.Spec.NodeLabelKeysToCopy, roleSuffix) if err != nil { return err } + for label, value := range node.BMHLabels { + labels[label] = value + } + bmh := &metal3.BareMetalHost{ ObjectMeta: metav1.ObjectMeta{ Name: bmhName, Namespace: getRuntimeNamespace(), // TODO add rack and server labels, when we crearly define mechanism // which labels we are copying - Labels: node.NodeLabel.VMFlavor, + Labels: labels, }, Spec: metal3.BareMetalHostSpec{ NetworkData: &corev1.SecretReference{ @@ -227,10 +230,13 @@ func (r *VinoReconciler) getBMHNodePrefix(vino *vinov1.Vino, pod corev1.Pod) str return fmt.Sprintf("%s-%s-%s", vino.Namespace, vino.Name, pod.Spec.NodeName) } -func (r *VinoReconciler) getBMCAddress( +func (r *VinoReconciler) getBMCAddressAndLabels( ctx context.Context, pod corev1.Pod, - vmName string) (string, error) { + labelKeys []string, + vmName string) (string, map[string]string, error) { + logger := logr.FromContext(ctx).WithValues("k8s node", pod.Spec.NodeName) + node := &corev1.Node{ ObjectMeta: metav1.ObjectMeta{ Name: pod.Spec.NodeName, @@ -238,15 +244,25 @@ func (r *VinoReconciler) getBMCAddress( } err := r.Get(ctx, client.ObjectKeyFromObject(node), node) if err != nil { - return "", err + return "", nil, err + } + + labels := map[string]string{} + + for _, key := range labelKeys { + value, ok := node.Labels[key] + if !ok { + logger.Info("Kubernetes node missing label from vino CR CopyNodeLabelKeys field", "label", key) + } + labels[key] = value } for _, addr := range node.Status.Addresses { if addr.Type == corev1.NodeInternalIP { - return fmt.Sprintf("redfish+http://%s:%d/redfish/v1/Systems/%s", addr.Address, 8000, vmName), nil + return fmt.Sprintf("redfish+http://%s:%d/redfish/v1/Systems/%s", addr.Address, 8000, vmName), labels, nil } } - return "", fmt.Errorf("Node %s doesn't have internal ip address defined", node.Name) + return "", labels, fmt.Errorf("Node %s doesn't have internal ip address defined", node.Name) } // reconcileBMHCredentials returns secret name with credentials and error diff --git a/pkg/controllers/bmh_test.go b/pkg/controllers/bmh_test.go index b12b36a..6fc7965 100644 --- a/pkg/controllers/bmh_test.go +++ b/pkg/controllers/bmh_test.go @@ -39,10 +39,18 @@ var _ = Describe("Test BMH reconciliation", func() { It("creates 6 BMH hosts", func() { os.Setenv("RUNTIME_NAMESPACE", "vino-system") defer os.Unsetenv("RUNTIME_NAMESPACE") + rackLabel := "airshipit.org/rack" + serverLabel := "airshipit.org/server" vino := testVINO() + providedFlavorLabel := "provided-label" + providedFlavorValue := "provided-value" + vino.Spec.NodeLabelKeysToCopy = []string{rackLabel, serverLabel} vino.Spec.Nodes = []vinov1.NodeSet{ { - Name: "worker", + Name: "worker", + BMHLabels: map[string]string{ + providedFlavorLabel: providedFlavorValue, + }, Count: 3, NetworkDataTemplate: vinov1.NamespacedName{ Name: "default-template", @@ -94,9 +102,16 @@ var _ = Describe("Test BMH reconciliation", func() { }, } + rack1 := "r1" + server1 := "s1" + node1Labels := map[string]string{ + rackLabel: rack1, + serverLabel: server1, + } node1 := &corev1.Node{ ObjectMeta: metav1.ObjectMeta{ - Name: "node01", + Name: "node01", + Labels: node1Labels, }, Status: corev1.NodeStatus{ Addresses: []corev1.NodeAddress{ @@ -148,6 +163,9 @@ var _ = Describe("Test BMH reconciliation", func() { Expect(reconciler.Get(ctx, client.ObjectKeyFromObject(bmh), bmh)).Should(Succeed()) Expect(bmh.Spec.BMC.Address).To(Equal("redfish+http://10.0.0.2:8000/redfish/v1/Systems/worker-1")) + Expect(bmh.Labels).To(HaveKeyWithValue(rackLabel, rack1)) + Expect(bmh.Labels).To(HaveKeyWithValue(serverLabel, server1)) + Expect(bmh.Labels).To(HaveKeyWithValue(providedFlavorLabel, providedFlavorValue)) Expect(reconciler.Get(ctx, client.ObjectKeyFromObject(networkSecret), networkSecret)).Should(Succeed()) Expect(networkSecret.StringData["networkData"]).To(Equal("REPLACEME")) }) diff --git a/tools/deployment/test-cr.sh b/tools/deployment/test-cr.sh index 9c2056b..b9f5c6d 100755 --- a/tools/deployment/test-cr.sh +++ b/tools/deployment/test-cr.sh @@ -12,6 +12,14 @@ function vinoDebugInfo () { exit 1 } +server_label="airshipit.org/server=s1" +rack_label="airshipit.org/rack=r1" +copyLabel="airshipit.org/k8s-role=worker" + +# Label all nodes with the same rack/label. We are ok with this for this simple test. +kubectl label node --overwrite=true --all $server_label $rack_label + + kubectl apply -f config/samples/vino_cr.yaml kubectl apply -f config/samples/ippool.yaml kubectl apply -f config/samples/network-template-secret.yaml @@ -44,12 +52,14 @@ if ! kubectl -n vino-system rollout status ds default-vino-test-cr --timeout=10s vinoDebugInfo fi -bmhCount=$(kubectl get baremetalhosts -n vino-system -o name | wc -l) +bmhCount=$(kubectl get baremetalhosts -n vino-system -l "$server_label,$server_label,$copyLabel" -o name | wc -l) # with this setup set up, exactly 3 BMHs must have been created by VINO controller [[ "$bmhCount" -eq "3" ]] +kubectl get baremetalhosts -n vino-system --show-labels=true + kubectl get -o yaml -n vino-system \ $(kubectl get secret -o name -n vino-system | grep network-data) kubectl get secret -o yaml -n vino-system default-vino-test-cr-credentials