From 85a2563e8f1ec786cbe034c66cacff261ffc50f6 Mon Sep 17 00:00:00 2001 From: rabi Date: Tue, 7 Jan 2025 08:40:30 +0530 Subject: [PATCH] WIP Don't set BaremetalHosts in the nodeset spec This is also probably required before we can change the BaremetalSetTemplate field. Jira: https://issues.redhat.com/browse/OSPRH-12709 Signed-off-by: rabi --- .../v1beta1/openstackdataplanenodeset_types.go | 5 ++--- .../v1beta1/openstackdataplanenodeset_webhook.go | 13 ++----------- hack/crd-schema-checker.sh | 8 ++++++-- pkg/dataplane/baremetal.go | 8 ++++---- .../openstackdataplanenodeset_controller_test.go | 6 +++--- 5 files changed, 17 insertions(+), 23 deletions(-) diff --git a/apis/dataplane/v1beta1/openstackdataplanenodeset_types.go b/apis/dataplane/v1beta1/openstackdataplanenodeset_types.go index bb2527700..34e362af0 100644 --- a/apis/dataplane/v1beta1/openstackdataplanenodeset_types.go +++ b/apis/dataplane/v1beta1/openstackdataplanenodeset_types.go @@ -188,9 +188,8 @@ func (instance *OpenStackDataPlaneNodeSet) InitConditions() { condition.UnknownCondition(condition.ServiceAccountReadyCondition, condition.InitReason, condition.ServiceAccountReadyInitMessage), ) - // Only set Baremetal related conditions if we have baremetal hosts included in the - // baremetalSetTemplate. - if len(instance.Spec.BaremetalSetTemplate.BaremetalHosts) > 0 { + // Only set Baremetal related conditions if required + if !instance.Spec.PreProvisioned && len(instance.Spec.Nodes) > 0 { cl = append(cl, *condition.UnknownCondition(NodeSetBareMetalProvisionReadyCondition, condition.InitReason, condition.InitReason)) } diff --git a/apis/dataplane/v1beta1/openstackdataplanenodeset_webhook.go b/apis/dataplane/v1beta1/openstackdataplanenodeset_webhook.go index 013fbe01f..f99d7e3ee 100644 --- a/apis/dataplane/v1beta1/openstackdataplanenodeset_webhook.go +++ b/apis/dataplane/v1beta1/openstackdataplanenodeset_webhook.go @@ -24,7 +24,6 @@ import ( "github.com/go-playground/validator/v10" "github.com/openstack-k8s-operators/lib-common/modules/common/condition" - baremetalv1 "github.com/openstack-k8s-operators/openstack-baremetal-operator/api/v1beta1" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" @@ -70,6 +69,7 @@ func (r *OpenStackDataPlaneNodeSet) Default() { // Default - set defaults for this OpenStackDataPlaneNodeSet Spec func (spec *OpenStackDataPlaneNodeSetSpec) Default() { domain := spec.BaremetalSetTemplate.DomainName + spec.BaremetalSetTemplate.BaremetalHosts = nil for nodeName, node := range spec.Nodes { if node.HostName == "" { node.HostName = nodeName @@ -79,6 +79,7 @@ func (spec *OpenStackDataPlaneNodeSetSpec) Default() { node.HostName = strings.Join([]string{nodeName, domain}, ".") } } + spec.Nodes[nodeName] = *node.DeepCopy() } @@ -87,15 +88,6 @@ func (spec *OpenStackDataPlaneNodeSetSpec) Default() { if spec.BaremetalSetTemplate.DeploymentSSHSecret == "" { spec.BaremetalSetTemplate.DeploymentSSHSecret = spec.NodeTemplate.AnsibleSSHPrivateKeySecret } - nodeSetHostMap := make(map[string]baremetalv1.InstanceSpec) - for _, node := range spec.Nodes { - instanceSpec := baremetalv1.InstanceSpec{} - instanceSpec.BmhLabelSelector = node.BmhLabelSelector - instanceSpec.UserData = node.UserData - instanceSpec.NetworkData = node.NetworkData - nodeSetHostMap[node.HostName] = instanceSpec - } - spec.BaremetalSetTemplate.BaremetalHosts = nodeSetHostMap } else if spec.NodeTemplate.Ansible.AnsibleUser == "" { spec.NodeTemplate.Ansible.AnsibleUser = "cloud-admin" } @@ -220,7 +212,6 @@ func (r *OpenStackDataPlaneNodeSetSpec) ValidateUpdate(oldSpec *OpenStackDataPla // If the BaremetalSetTemplate is changed, we will offload the parsing of these details // to the openstack-baremetal-operator webhook to avoid duplicating logic. if !reflect.DeepEqual(r.BaremetalSetTemplate, oldSpec.BaremetalSetTemplate) { - // Call openstack-baremetal-operator webhook Validate() to parse changes err := r.BaremetalSetTemplate.Validate(oldSpec.BaremetalSetTemplate) if err != nil { diff --git a/hack/crd-schema-checker.sh b/hack/crd-schema-checker.sh index ee105c734..25ac2f236 100755 --- a/hack/crd-schema-checker.sh +++ b/hack/crd-schema-checker.sh @@ -3,11 +3,15 @@ set -euxo pipefail CHECKER=$INSTALL_DIR/crd-schema-checker -DISABLED_VALIDATORS=NoMaps # TODO: https://issues.redhat.com/browse/OSPRH-12254 +# (TODO) Remove NoFieldRemoval after this PR merges +DISABLED_VALIDATORS=NoMaps,NoFieldRemoval # TODO: https://issues.redhat.com/browse/OSPRH-12254 CHECKER_ARGS="" if [[ ${DISABLED_VALIDATORS:+x} ]]; then - CHECKER_ARGS="$CHECKER_ARGS --disabled-validators $DISABLED_VALIDATORS" + CHECKER_ARGS="$CHECKER_ARGS " + for check in ${DISABLED_VALIDATORS//,/ }; do + CHECKER_ARGS+=" --disabled-validators $check" + done fi TMP_DIR=$(mktemp -d) diff --git a/pkg/dataplane/baremetal.go b/pkg/dataplane/baremetal.go index acf45ff2e..8f8e9e5da 100644 --- a/pkg/dataplane/baremetal.go +++ b/pkg/dataplane/baremetal.go @@ -49,9 +49,6 @@ func DeployBaremetalSet( }, } - if instance.Spec.BaremetalSetTemplate.BaremetalHosts == nil { - return false, fmt.Errorf("no baremetal hosts set in baremetalSetTemplate") - } utils.LogForObject(helper, "Reconciling BaremetalSet", instance) _, err := controllerutil.CreateOrPatch(ctx, helper.GetClient(), baremetalSet, func() error { ownerLabels := labels.GetLabels(instance, labels.GetGroupLabel(NodeSetLabel), map[string]string{}) @@ -72,11 +69,14 @@ func DeployBaremetalSet( for _, node := range instance.Spec.Nodes { hostName := node.HostName ipSet, ok := ipSets[hostName] - instanceSpec := baremetalSet.Spec.BaremetalHosts[hostName] if !ok { err := fmt.Errorf("no IPSet found for host: %s", hostName) return err } + instanceSpec := baremetalv1.InstanceSpec{} + instanceSpec.BmhLabelSelector = node.BmhLabelSelector + instanceSpec.UserData = node.UserData + instanceSpec.NetworkData = node.NetworkData for _, res := range ipSet.Status.Reservation { if strings.ToLower(string(res.Network)) == dataplanev1.CtlPlaneNetwork { _, ipNet, err := net.ParseCIDR(res.Cidr) diff --git a/tests/functional/dataplane/openstackdataplanenodeset_controller_test.go b/tests/functional/dataplane/openstackdataplanenodeset_controller_test.go index 3028168d2..f24a80b93 100644 --- a/tests/functional/dataplane/openstackdataplanenodeset_controller_test.go +++ b/tests/functional/dataplane/openstackdataplanenodeset_controller_test.go @@ -339,8 +339,8 @@ var _ = Describe("Dataplane NodeSet Test", func() { "nova", "telemetry"} - Expect(dataplaneNodeSetInstance.Spec.BaremetalSetTemplate.BaremetalHosts).Should(BeEmpty()) Expect(dataplaneNodeSetInstance.Spec.Services).Should(Equal(services)) + Expect(dataplaneNodeSetInstance.Spec.BaremetalSetTemplate.BaremetalHosts).Should(BeEmpty()) }) It("should have input not ready and unknown Conditions initialized", func() { @@ -448,8 +448,8 @@ var _ = Describe("Dataplane NodeSet Test", func() { "telemetry", "global-service"} - Expect(dataplaneNodeSetInstance.Spec.BaremetalSetTemplate.BaremetalHosts).Should(BeEmpty()) Expect(dataplaneNodeSetInstance.Spec.Services).Should(Equal(services)) + Expect(dataplaneNodeSetInstance.Spec.BaremetalSetTemplate.BaremetalHosts).Should(BeEmpty()) }) It("should have input not ready and unknown Conditions initialized", func() { @@ -870,8 +870,8 @@ var _ = Describe("Dataplane NodeSet Test", func() { "nova", "telemetry"} - Expect(dataplaneNodeSetInstance.Spec.BaremetalSetTemplate.BaremetalHosts).Should(BeEmpty()) Expect(dataplaneNodeSetInstance.Spec.Services).Should(Equal(services)) + Expect(dataplaneNodeSetInstance.Spec.BaremetalSetTemplate.BaremetalHosts).Should(BeEmpty()) }) It("should have input not ready and unknown Conditions initialized", func() { th.ExpectCondition(