From 5551ec347a4241662e9fcee57e3f888cdded7f59 Mon Sep 17 00:00:00 2001 From: Kostiantyn Kalynovskyi Date: Thu, 8 Apr 2021 00:02:34 +0000 Subject: [PATCH] Pass dynamic IPAM information to vino-buider This patchset enables VINO controller to pass variables to vino-builder that are generated dynamically by IPAM module. Also it takes IP address of the vino bridge and injects it into node annotation togather with IPAM values. Vino-builder polls a k8s node object to get these values before proceeding. Change-Id: I5b4e23df0fa4fa980b2a6724468bc6f2d9546409 --- .../bases/airship.airshipit.org_vinoes.yaml | 2 +- config/manager/flavor-templates.yaml | 23 +- config/rbac/role.yaml | 2 + config/samples/vino_cr.yaml | 4 +- docs/api/vino.md | 120 ++++++++- pkg/api/v1/vino_builder.go | 35 +++ pkg/api/v1/vino_types.go | 6 +- pkg/api/v1/zz_generated.deepcopy.go | 81 ++++++ pkg/controllers/bmh.go | 248 +++++++++++++----- pkg/controllers/bmh_test.go | 8 +- pkg/controllers/vino_controller.go | 42 ++- .../tasks/main.yaml | 12 +- 12 files changed, 475 insertions(+), 108 deletions(-) create mode 100644 pkg/api/v1/vino_builder.go diff --git a/config/crd/bases/airship.airshipit.org_vinoes.yaml b/config/crd/bases/airship.airshipit.org_vinoes.yaml index dde5aae..1b5b764 100644 --- a/config/crd/bases/airship.airshipit.org_vinoes.yaml +++ b/config/crd/bases/airship.airshipit.org_vinoes.yaml @@ -178,7 +178,7 @@ spec: type: string type: object name: - description: Parameter for Node master or worker + description: Parameter for Node control-plane or worker type: string networkDataTemplate: description: NetworkDataTemplate must have a template key diff --git a/config/manager/flavor-templates.yaml b/config/manager/flavor-templates.yaml index 085d72b..cb1f3f0 100644 --- a/config/manager/flavor-templates.yaml +++ b/config/manager/flavor-templates.yaml @@ -2,6 +2,8 @@ flavorTemplates: master: domainTemplate: | {% set nodename = 'master-' + item|string %} + {% if domains[nodename] is defined %} + {% set domain = domains[nodename] %} {{ nodename }} {{ nodename | hash('md5') }} @@ -71,12 +73,14 @@ flavorTemplates: # for each interface defined in vino, e.g. + {% for if_name, if_values in domain.interfaces.items() %} - - + + -
+
+ {% endfor %} @@ -103,6 +107,7 @@ flavorTemplates: +42424:+104 + {% endif %} volumeTemplate: | {% set nodename = 'master-' + item|string %} @@ -116,6 +121,8 @@ flavorTemplates: worker: domainTemplate: | {% set nodename = 'worker-' + item|string %} + {% if domains[nodename] is defined %} + {% set domain = domains[nodename] %} {{ nodename }} {{ nodename | hash('md5') }} @@ -184,13 +191,14 @@ flavorTemplates:
- # for each interface defined in vino, e.g. + {% for if_name, if_values in domain.interfaces.items() %} - - + + -
+
+ {% endfor %} @@ -217,6 +225,7 @@ flavorTemplates: +42424:+104 + {% endif %} volumeTemplate: | {% set nodename = 'worker-' + item|string %} diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index c5316c2..71321d6 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -13,6 +13,8 @@ rules: verbs: - get - list + - patch + - update - watch - apiGroups: - "" diff --git a/config/samples/vino_cr.yaml b/config/samples/vino_cr.yaml index 3d42e8e..7cbaccd 100644 --- a/config/samples/vino_cr.yaml +++ b/config/samples/vino_cr.yaml @@ -4,6 +4,7 @@ metadata: name: vino-test-cr labels: {} spec: + vmBridge: lo nodeLabelKeysToCopy: - "airshipit.org/server" - "airshipit.org/rack" @@ -24,7 +25,7 @@ spec: routes: - network: 10.0.0.0 netmask: 255.255.255.0 - gateway: 192.168.2.1 # vino will need to populate this from the nodelabel value `airshipit.org/vino.nodebridgegw` + gateway: $vinobridge # vino will need to populate this from the nodelabel value `airshipit.org/vino.nodebridgegw` dns_servers: ["135.188.34.124"] - name: external subnet: 169.0.0.0/24 @@ -36,7 +37,6 @@ spec: allocationStart: 169.0.0.10 allocationStop: 169.0.0.254 macPrefix: "0A:00:00:00:00:00" - vmBridge: lo nodes: - name: master count: 1 diff --git a/docs/api/vino.md b/docs/api/vino.md index 9ffe277..af66984 100644 --- a/docs/api/vino.md +++ b/docs/api/vino.md @@ -102,6 +102,124 @@ string +

Builder +

+
+
+ + + + + + + + + + + + + + + + + + + + + +
FieldDescription
+gwIPBridge
+ +string + +		 +
+networks
+ + +map[string]./pkg/api/v1.BuilderNetwork + + +		 +
+domains
+ + +map[string]./pkg/api/v1.BuilderDomain + + +		 +
+
+
+

BuilderDomain +

+

+(Appears on: +Builder) +

+
+
+ + + + + + + + + + + + + +
FieldDescription
+interfaces
+ + +map[string]./pkg/api/v1.BuilderNetworkInterface + + +		 +
+
+
+

BuilderNetwork +

+

+(Appears on: +Builder) +

+

BuilderNetworkInterface +

+

+(Appears on: +BuilderDomain) +

+
+
+ + + + + + + + + + + + + +
FieldDescription
+macAddress
+ +string + +		 +
+
+

CPUConfiguration

@@ -796,7 +914,7 @@ string -

Parameter for Node master or worker

+

Parameter for Node control-plane or worker

diff --git a/pkg/api/v1/vino_builder.go b/pkg/api/v1/vino_builder.go new file mode 100644 index 0000000..7d31777 --- /dev/null +++ b/pkg/api/v1/vino_builder.go @@ -0,0 +1,35 @@ +/* + + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package v1 + +type Builder struct { + GWIPBridge string `json:"gwIPBridge,omitempty"` + Networks map[string]BuilderNetwork `json:"networks,omitempty"` + Domains map[string]BuilderDomain `json:"domains,omitempty"` +} + +type BuilderNetworkInterface struct { + MACAddress string `json:"macAddress,omitempty"` +} + +type BuilderNetwork struct { + // Placeholder for future development +} + +type BuilderDomain struct { + Interfaces map[string]BuilderNetworkInterface `json:"interfaces,omitempty"` +} diff --git a/pkg/api/v1/vino_types.go b/pkg/api/v1/vino_types.go index c2d1a77..48551d0 100644 --- a/pkg/api/v1/vino_types.go +++ b/pkg/api/v1/vino_types.go @@ -33,6 +33,10 @@ const ( VinoFinalizer = "vino.airshipit.org" // EnvVarVMInterfaceName environment variable that is used to find VM interface to use for vms EnvVarVMInterfaceName = "VM_BRIDGE_INTERFACE" + // VinoDefaultGatewayBridgeLabel is used to identify ip address of the default gateway for the VM + VinoDefaultGatewayBridgeLabel = "airshipit.org/vino.nodebridgegw" + // VinoNodeNetworkValuesAnnotation vino controller saves ip and mac address information for the node in it + VinoNodeNetworkValuesAnnotation = "airshipit.org/vino.network-values" ) // VinoSpec defines the desired state of Vino @@ -104,7 +108,7 @@ type VMRoutes struct { //NodeSet node definitions type NodeSet struct { - //Parameter for Node master or worker + // Parameter for Node control-plane or worker Name string `json:"name,omitempty"` Count int `json:"count,omitempty"` // BMHLabels labels will be copied directly to BMHs that will be created diff --git a/pkg/api/v1/zz_generated.deepcopy.go b/pkg/api/v1/zz_generated.deepcopy.go index 59d4c40..a97e2fa 100644 --- a/pkg/api/v1/zz_generated.deepcopy.go +++ b/pkg/api/v1/zz_generated.deepcopy.go @@ -55,6 +55,87 @@ func (in *BMCCredentials) DeepCopy() *BMCCredentials { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *Builder) DeepCopyInto(out *Builder) { + *out = *in + if in.Networks != nil { + in, out := &in.Networks, &out.Networks + *out = make(map[string]BuilderNetwork, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } + if in.Domains != nil { + in, out := &in.Domains, &out.Domains + *out = make(map[string]BuilderDomain, len(*in)) + for key, val := range *in { + (*out)[key] = *val.DeepCopy() + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Builder. +func (in *Builder) DeepCopy() *Builder { + if in == nil { + return nil + } + out := new(Builder) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *BuilderDomain) DeepCopyInto(out *BuilderDomain) { + *out = *in + if in.Interfaces != nil { + in, out := &in.Interfaces, &out.Interfaces + *out = make(map[string]BuilderNetworkInterface, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new BuilderDomain. +func (in *BuilderDomain) DeepCopy() *BuilderDomain { + if in == nil { + return nil + } + out := new(BuilderDomain) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *BuilderNetwork) DeepCopyInto(out *BuilderNetwork) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new BuilderNetwork. +func (in *BuilderNetwork) DeepCopy() *BuilderNetwork { + if in == nil { + return nil + } + out := new(BuilderNetwork) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *BuilderNetworkInterface) DeepCopyInto(out *BuilderNetworkInterface) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new BuilderNetworkInterface. +func (in *BuilderNetworkInterface) DeepCopy() *BuilderNetworkInterface { + if in == nil { + return nil + } + out := new(BuilderNetworkInterface) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *CPUConfiguration) DeepCopyInto(out *CPUConfiguration) { *out = *in diff --git a/pkg/controllers/bmh.go b/pkg/controllers/bmh.go index fe0cec5..7fd109c 100644 --- a/pkg/controllers/bmh.go +++ b/pkg/controllers/bmh.go @@ -19,6 +19,7 @@ import ( "context" "fmt" "text/template" + "time" "github.com/Masterminds/sprig" "github.com/go-logr/logr" @@ -29,6 +30,7 @@ import ( "k8s.io/apimachinery/pkg/types" kerror "k8s.io/apimachinery/pkg/util/errors" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/yaml" vinov1 "vino/pkg/api/v1" "vino/pkg/ipam" @@ -48,8 +50,8 @@ type networkTemplateValues struct { } type generatedValues struct { - IPAddresses map[string]string // a map of network names to IP addresses - MACAddresses map[string]string // a map of network interface (link) names to MACs + IPAddresses map[string]string + MACAddresses map[string]string } func (r *VinoReconciler) ensureBMHs(ctx context.Context, vino *vinov1.Vino) error { @@ -130,12 +132,13 @@ func (r *VinoReconciler) createIpamNetworks(ctx context.Context, vino *vinov1.Vi if err != nil { return err } - if network.MACPrefix == "" { - logger.Info("No MACPrefix provided; using default MACPrefix %s for network %s", - DefaultMACPrefix, network.Name) - network.MACPrefix = DefaultMACPrefix + macPrefix := network.MACPrefix + if macPrefix == "" { + logger.Info("No MACPrefix provided; using default MACPrefix for network", + "default prefix", DefaultMACPrefix, "network name", network.Name) + macPrefix = DefaultMACPrefix } - err = r.Ipam.AddSubnetRange(ctx, network.SubNet, subnetRange, network.MACPrefix) + err = r.Ipam.AddSubnetRange(ctx, network.SubNet, subnetRange, macPrefix) if err != nil { return err } @@ -145,6 +148,19 @@ func (r *VinoReconciler) createIpamNetworks(ctx context.Context, vino *vinov1.Vi func (r *VinoReconciler) createBMHperPod(ctx context.Context, vino *vinov1.Vino, pod corev1.Pod) error { logger := logr.FromContext(ctx) + + nodeNetworkValues := map[string]generatedValues{} + + k8sNode, err := r.getNode(ctx, pod) + if err != nil { + return err + } + + ip, err := r.getBridgeIP(ctx, k8sNode) + if err != nil { + return err + } + for _, node := range vino.Spec.Nodes { logger.Info("Creating BMHs for vino node", "node name", node.Name, "count", node.Count) prefix := r.getBMHNodePrefix(vino, pod) @@ -152,58 +168,25 @@ func (r *VinoReconciler) createBMHperPod(ctx context.Context, vino *vinov1.Vino, roleSuffix := fmt.Sprintf("%s-%d", node.Name, i) bmhName := fmt.Sprintf("%s-%s", prefix, roleSuffix) - creds, err := r.reconcileBMHCredentials(ctx, vino) - if err != nil { - return err + creds, nodeErr := r.reconcileBMHCredentials(ctx, vino) + if nodeErr != nil { + return nodeErr } - // Allocate an IP for each of this BMH's network interfaces - ipAddresses := map[string]string{} - macAddresses := map[string]string{} - for _, iface := range node.NetworkInterfaces { - networkName := iface.NetworkName - subnet := "" - subnetRange := vinov1.Range{} - for _, network := range vino.Spec.Networks { - if network.Name == networkName { - subnet = network.SubNet - subnetRange, err = ipam.NewRange(network.AllocationStart, - network.AllocationStop) - if err != nil { - return err - } - break - } - } - if subnet == "" { - return fmt.Errorf("Interface %s doesn't have a matching network defined", networkName) - } - ipAllocatedTo := fmt.Sprintf("%s/%s", bmhName, iface.NetworkName) - ipAddress, macAddress, er := r.Ipam.AllocateIP(ctx, subnet, subnetRange, ipAllocatedTo) - if er != nil { - return er - } - ipAddresses[networkName] = ipAddress - macAddresses[iface.Name] = macAddress + values, nodeErr := r.networkValues(ctx, bmhName, ip, node, vino) + if nodeErr != nil { + return nodeErr + } + nodeNetworkValues[roleSuffix] = values.Generated + + netData, netDataNs, nodeErr := r.reconcileBMHNetworkData(ctx, node, vino, values) + if nodeErr != nil { + return nodeErr } - values := networkTemplateValues{ - Node: node, - BMHName: bmhName, - Networks: vino.Spec.Networks, - Generated: generatedValues{ - IPAddresses: ipAddresses, - MACAddresses: macAddresses, - }, - } - netData, netDataNs, err := r.reconcileBMHNetworkData(ctx, node, vino, values) - if err != nil { - return err - } - - bmcAddr, labels, err := r.getBMCAddressAndLabels(ctx, pod, vino.Spec.NodeLabelKeysToCopy, roleSuffix) - if err != nil { - return err + bmcAddr, labels, nodeErr := r.getBMCAddressAndLabels(ctx, k8sNode, vino.Spec.NodeLabelKeysToCopy, roleSuffix) + if nodeErr != nil { + return nodeErr } for label, value := range node.BMHLabels { @@ -232,15 +215,147 @@ func (r *VinoReconciler) createBMHperPod(ctx context.Context, vino *vinov1.Vino, } objKey := client.ObjectKeyFromObject(bmh) logger.Info("Creating BMH", "name", objKey) - err = applyRuntimeObject(ctx, objKey, bmh, r.Client) - if err != nil { - return err + nodeErr = applyRuntimeObject(ctx, objKey, bmh, r.Client) + if nodeErr != nil { + return nodeErr } } } + logger.Info("annotating node", "node", k8sNode.Name) + if err = r.annotateNode(ctx, ip, k8sNode, nodeNetworkValues); err != nil { + return err + } return nil } +func (r *VinoReconciler) networkValues( + ctx context.Context, + bmhName string, + bridgeIP string, + node vinov1.NodeSet, + vino *vinov1.Vino) (networkTemplateValues, error) { + // Allocate an IP for each of this BMH's network interfaces + ipAddresses := map[string]string{} + macAddresses := map[string]string{} + for _, iface := range node.NetworkInterfaces { + networkName := iface.NetworkName + subnet := "" + var err error + subnetRange := vinov1.Range{} + for netIndex, network := range vino.Spec.Networks { + for routeIndex, route := range network.Routes { + if route.Gateway == "$vinobridge" { + vino.Spec.Networks[netIndex].Routes[routeIndex].Gateway = bridgeIP + } + } + if network.Name == networkName { + subnet = network.SubNet + subnetRange, err = ipam.NewRange(network.AllocationStart, + network.AllocationStop) + if err != nil { + return networkTemplateValues{}, err + } + break + } + } + if subnet == "" { + return networkTemplateValues{}, fmt.Errorf("Interface %s doesn't have a matching network defined", networkName) + } + ipAllocatedTo := fmt.Sprintf("%s/%s", bmhName, iface.NetworkName) + ipAddress, macAddress, err := r.Ipam.AllocateIP(ctx, subnet, subnetRange, ipAllocatedTo) + if err != nil { + return networkTemplateValues{}, err + } + ipAddresses[networkName] = ipAddress + macAddresses[iface.Name] = macAddress + logr.FromContext(ctx).Info("Got MAC and IP for the network and node", + "MAC", macAddress, "IP", ipAddress, "bmh name", bmhName) + } + return networkTemplateValues{ + Node: node, + BMHName: bmhName, + Networks: vino.Spec.Networks, + Generated: generatedValues{ + IPAddresses: ipAddresses, + MACAddresses: macAddresses, + }, + }, nil +} + +func (r *VinoReconciler) annotateNode(ctx context.Context, + gwIP string, + k8sNode *corev1.Node, + values map[string]generatedValues) error { + logr.FromContext(ctx).Info("Getting GW bridge IP from node", "node", k8sNode.Name) + builderValues := vinov1.Builder{ + Domains: make(map[string]vinov1.BuilderDomain), + GWIPBridge: gwIP, + } + for domainName, domain := range values { + builderDomain := vinov1.BuilderDomain{ + Interfaces: make(map[string]vinov1.BuilderNetworkInterface), + } + for ifName, ifMAC := range domain.MACAddresses { + builderDomain.Interfaces[ifName] = vinov1.BuilderNetworkInterface{ + MACAddress: ifMAC, + } + } + builderValues.Domains[domainName] = builderDomain + } + + b, err := yaml.Marshal(builderValues) + if err != nil { + return err + } + + annotations := k8sNode.GetAnnotations() + if k8sNode.GetAnnotations() == nil { + annotations = make(map[string]string) + } + + annotations[vinov1.VinoNodeNetworkValuesAnnotation] = string(b) + k8sNode.SetAnnotations(annotations) + + return r.Update(ctx, k8sNode) +} + +func (r *VinoReconciler) getBridgeIP(ctx context.Context, k8sNode *corev1.Node) (string, error) { + ctxTimeout, cancel := context.WithTimeout(ctx, 30*time.Second) + defer cancel() + + for { + select { + case <-ctxTimeout.Done(): + return "", ctx.Err() + default: + node := &corev1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: k8sNode.Name, + }, + } + if err := r.Get(ctx, client.ObjectKeyFromObject(node), node); err != nil { + return "", err + } + + ip, exist := k8sNode.Labels[vinov1.VinoDefaultGatewayBridgeLabel] + if exist { + return ip, nil + } + time.Sleep(10 * time.Second) + } + } +} + +func (r *VinoReconciler) getNode(ctx context.Context, pod corev1.Pod) (*corev1.Node, error) { + node := &corev1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: pod.Spec.NodeName, + }, + } + err := r.Get(ctx, client.ObjectKeyFromObject(node), node) + return node, err +} + func (r *VinoReconciler) getBMHNodePrefix(vino *vinov1.Vino, pod corev1.Pod) string { // TODO we need to do something about name length limitations return fmt.Sprintf("%s-%s-%s", vino.Namespace, vino.Name, pod.Spec.NodeName) @@ -248,20 +363,10 @@ func (r *VinoReconciler) getBMHNodePrefix(vino *vinov1.Vino, pod corev1.Pod) str func (r *VinoReconciler) getBMCAddressAndLabels( ctx context.Context, - pod corev1.Pod, + node *corev1.Node, 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, - }, - } - err := r.Get(ctx, client.ObjectKeyFromObject(node), node) - if err != nil { - return "", nil, err - } + logger := logr.FromContext(ctx).WithValues("k8s node", node.Name) labels := map[string]string{} @@ -336,6 +441,8 @@ func (r *VinoReconciler) reconcileBMHNetworkData( return "", "", err } + logger.Info("Genereated MAC Addresses values are", "GENERATED VALUES", values.Generated.MACAddresses) + buf := bytes.NewBuffer([]byte{}) err = tpl.Execute(buf, values) if err != nil { @@ -343,7 +450,6 @@ func (r *VinoReconciler) reconcileBMHNetworkData( } name := fmt.Sprintf("%s-network-data", values.BMHName) - ns := getRuntimeNamespace() netSecret := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ diff --git a/pkg/controllers/bmh_test.go b/pkg/controllers/bmh_test.go index 6fc7965..724852c 100644 --- a/pkg/controllers/bmh_test.go +++ b/pkg/controllers/bmh_test.go @@ -110,8 +110,9 @@ var _ = Describe("Test BMH reconciliation", func() { } node1 := &corev1.Node{ ObjectMeta: metav1.ObjectMeta{ - Name: "node01", - Labels: node1Labels, + Name: "node01", + Labels: node1Labels, + Annotations: make(map[string]string), }, Status: corev1.NodeStatus{ Addresses: []corev1.NodeAddress{ @@ -124,7 +125,8 @@ var _ = Describe("Test BMH reconciliation", func() { } node2 := &corev1.Node{ ObjectMeta: metav1.ObjectMeta{ - Name: "node02", + Name: "node02", + Annotations: make(map[string]string), }, Status: corev1.NodeStatus{ Addresses: []corev1.NodeAddress{ diff --git a/pkg/controllers/vino_controller.go b/pkg/controllers/vino_controller.go index ccabe3a..fdc69ec 100644 --- a/pkg/controllers/vino_controller.go +++ b/pkg/controllers/vino_controller.go @@ -61,7 +61,7 @@ type VinoReconciler struct { // +kubebuilder:rbac:groups=airship.airshipit.org,resources=vinoes/status,verbs=get;update;patch // +kubebuilder:rbac:groups=airship.airshipit.org,resources=ippools,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups="",resources=pods,verbs=list;watch -// +kubebuilder:rbac:groups="",resources=nodes,verbs=get;list;watch +// +kubebuilder:rbac:groups="",resources=nodes,verbs=get;list;watch;update;patch // +kubebuilder:rbac:groups="",resources=secrets,verbs=get;list;watch;create;update;patch func (r *VinoReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { @@ -109,11 +109,6 @@ func (r *VinoReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl. return ctrl.Result{Requeue: true}, err } - err = r.reconcileBMHs(ctx, vino) - if err != nil { - return ctrl.Result{Requeue: true}, err - } - vinov1.VinoReady(vino) if err := r.patchStatus(ctx, vino); err != nil { err = fmt.Errorf("unable to patch status after reconciliation: %w", err) @@ -320,10 +315,24 @@ func (r *VinoReconciler) ensureDaemonSet(ctx context.Context, vino *vinov1.Vino) // controller should watch for changes in daemonset to reconcile if it breaks, and change status // of the vino object // controlleruti.SetControllerReference(vino, ds, r.scheme) - ctx, cancel := context.WithTimeout(ctx, time.Second*180) + scheduledTimeoutCtx, cancel := context.WithTimeout(ctx, time.Second*180) defer cancel() - return r.waitDaemonSet(ctx, ds) + logger := logr.FromContext(ctx) + logger.Info("Waiting for daemonset to become scheduled") + if err = r.waitDaemonSet(scheduledTimeoutCtx, dsScheduled, ds); err != nil { + return err + } + + if err = r.reconcileBMHs(ctx, vino); err != nil { + return err + } + + waitTimeoutCtx, cancel := context.WithTimeout(ctx, time.Second*180) + defer cancel() + + logger.Info("Waiting for daemonset to become ready") + return r.waitDaemonSet(waitTimeoutCtx, dsReady, ds) } func (r *VinoReconciler) decorateDaemonSet(ctx context.Context, ds *appsv1.DaemonSet, vino *vinov1.Vino) { @@ -414,7 +423,7 @@ func setEnv(ctx context.Context, ds *appsv1.DaemonSet, vino *vinov1.Vino) { } } -func (r *VinoReconciler) waitDaemonSet(ctx context.Context, ds *appsv1.DaemonSet) error { +func (r *VinoReconciler) waitDaemonSet(ctx context.Context, check dsWaitCondition, ds *appsv1.DaemonSet) error { logger := logr.FromContext(ctx).WithValues( "daemonset", ds.Namespace+"/"+ds.Name) for { @@ -429,12 +438,11 @@ func (r *VinoReconciler) waitDaemonSet(ctx context.Context, ds *appsv1.DaemonSet Namespace: ds.Namespace, }, getDS) if err != nil { - logger.Info("received error while waiting for ds to become ready, sleeping", + logger.Info("received error while waiting for ds to reach desired condition, sleeping", "error", err.Error()) } else { logger.Info("checking daemonset status", "status", getDS.Status) - if getDS.Status.DesiredNumberScheduled == getDS.Status.NumberReady && - getDS.Status.DesiredNumberScheduled != 0 { + if check(getDS) { logger.Info("DaemonSet is in ready status") return nil } @@ -527,3 +535,13 @@ func applyRuntimeObject(ctx context.Context, key client.ObjectKey, obj client.Ob func getRuntimeNamespace() string { return os.Getenv("RUNTIME_NAMESPACE") } + +func dsScheduled(ds *appsv1.DaemonSet) bool { + return ds.Status.DesiredNumberScheduled != 0 && ds.Status.DesiredNumberScheduled == ds.Status.CurrentNumberScheduled +} + +func dsReady(ds *appsv1.DaemonSet) bool { + return ds.Status.DesiredNumberScheduled != 0 && ds.Status.DesiredNumberScheduled == ds.Status.NumberReady +} + +type dsWaitCondition func(ds *appsv1.DaemonSet) bool diff --git a/roles/vino-describe-kubernetes-objects/tasks/main.yaml b/roles/vino-describe-kubernetes-objects/tasks/main.yaml index e90fc6e..c6c1591 100644 --- a/roles/vino-describe-kubernetes-objects/tasks/main.yaml +++ b/roles/vino-describe-kubernetes-objects/tasks/main.yaml @@ -28,25 +28,17 @@ collect_namespaced_objects: - baremetalhosts - configmaps - - cronjobs - daemonsets - deployments - - endpoints - - ingresses - - jobs - - networkpolicies - pods - - podsecuritypolicies - - persistentvolumeclaims - replicasets - rolebindings - roles - secrets - serviceaccounts - - services - - statefulsets - vino - + - nodes + - ippool - name: "Get context list" include_tasks: get-contexts.yaml