diff --git a/apis/dataplane/v1beta1/openstackdataplanenodeset_types.go b/apis/dataplane/v1beta1/openstackdataplanenodeset_types.go index 83e3a9bb5..bc421be6e 100644 --- a/apis/dataplane/v1beta1/openstackdataplanenodeset_types.go +++ b/apis/dataplane/v1beta1/openstackdataplanenodeset_types.go @@ -268,25 +268,33 @@ func getStrPtr(in string) *string { // duplicateNodeCheck checks the NodeSetList for pre-existing nodes. If the user is trying to redefine an // existing node, we will return an error and block resource creation. -func (r *OpenStackDataPlaneNodeSetSpec) duplicateNodeCheck(nodeSetList *OpenStackDataPlaneNodeSetList) (errors field.ErrorList) { +func (r *OpenStackDataPlaneNodeSetSpec) duplicateNodeCheck(nodeSetList *OpenStackDataPlaneNodeSetList, nodesetName string) (errors field.ErrorList) { existingNodeNames := make([]string, 0) - for _, existingNode := range nodeSetList.Items { - for _, node := range existingNode.Spec.Nodes { + for _, nodeSet := range nodeSetList.Items { + if nodeSet.ObjectMeta.Name == nodesetName { + continue + } + for _, node := range nodeSet.Spec.Nodes { existingNodeNames = append(existingNodeNames, node.HostName) if node.Ansible.AnsibleHost != "" { existingNodeNames = append(existingNodeNames, node.Ansible.AnsibleHost) } } + } - for _, newNodeName := range r.Nodes { - if slices.Contains(existingNodeNames, newNodeName.HostName) || slices.Contains(existingNodeNames, newNodeName.Ansible.AnsibleHost) { + for _, newNode := range r.Nodes { + if slices.Contains(existingNodeNames, newNode.HostName) || slices.Contains(existingNodeNames, newNode.Ansible.AnsibleHost) { errors = append(errors, field.Invalid( field.NewPath("spec").Child("nodes"), - newNodeName, - fmt.Sprintf("node %s already exists in another cluster", newNodeName.HostName))) + newNode, + fmt.Sprintf("node %s already exists in another cluster", newNode.HostName))) + } else { + existingNodeNames = append(existingNodeNames, newNode.HostName) + if newNode.Ansible.AnsibleHost != "" { + existingNodeNames = append(existingNodeNames, newNode.Ansible.AnsibleHost) + } } } - return } diff --git a/apis/dataplane/v1beta1/openstackdataplanenodeset_webhook.go b/apis/dataplane/v1beta1/openstackdataplanenodeset_webhook.go index a58f1b3fb..13d6e3291 100644 --- a/apis/dataplane/v1beta1/openstackdataplanenodeset_webhook.go +++ b/apis/dataplane/v1beta1/openstackdataplanenodeset_webhook.go @@ -108,30 +108,21 @@ var _ webhook.Validator = &OpenStackDataPlaneNodeSet{} // ValidateCreate implements webhook.Validator so a webhook will be registered for the type func (r *OpenStackDataPlaneNodeSet) ValidateCreate() (admission.Warnings, error) { openstackdataplanenodesetlog.Info("validate create", "name", r.Name) - var errors field.ErrorList - - nodeSetList := &OpenStackDataPlaneNodeSetList{} - opts := &client.ListOptions{ - Namespace: r.ObjectMeta.Namespace, - } - - err := webhookClient.List(context.TODO(), nodeSetList, opts) + errors, err := r.validateNodes() if err != nil { return nil, err } + // Check if OpenStackDataPlaneNodeSet name matches RFC1123 for use in labels validate := validator.New() - if err = validate.Var(r.Name, "hostname_rfc1123"); err != nil { + if err := validate.Var(r.Name, "hostname_rfc1123"); err != nil { openstackdataplanenodesetlog.Error(err, "Error validating OpenStackDataPlaneNodeSet name, name must follow RFC1123") errors = append(errors, field.Invalid( field.NewPath("Name"), r.Name, fmt.Sprintf("Error validating OpenStackDataPlaneNodeSet name %s, name must follow RFC1123", r.Name))) } - - errors = append(errors, r.Spec.ValidateCreate(nodeSetList)...) - if len(errors) > 0 { openstackdataplanenodesetlog.Info("validation failed", "name", r.Name) @@ -143,17 +134,27 @@ func (r *OpenStackDataPlaneNodeSet) ValidateCreate() (admission.Warnings, error) return nil, nil } -func (r *OpenStackDataPlaneNodeSetSpec) ValidateCreate(nodeSetList *OpenStackDataPlaneNodeSetList) field.ErrorList { +func (r *OpenStackDataPlaneNodeSet) validateNodes() (field.ErrorList, error) { var errors field.ErrorList + nodeSetList := &OpenStackDataPlaneNodeSetList{} + opts := &client.ListOptions{ + Namespace: r.ObjectMeta.Namespace, + } + + err := webhookClient.List(context.TODO(), nodeSetList, opts) + if err != nil { + return nil, err + } + // Currently, this check is only valid for PreProvisioned nodes. Since we can't possibly // have duplicates in Baremetal Deployments, we can exit early here for Baremetal NodeSets. // If this is the first NodeSet being created, then there can be no duplicates // we can exit early here. - if r.PreProvisioned && len(nodeSetList.Items) != 0 { - errors = append(errors, r.duplicateNodeCheck(nodeSetList)...) + if r.Spec.PreProvisioned && len(nodeSetList.Items) != 0 { + errors = append(errors, r.Spec.duplicateNodeCheck(nodeSetList, r.ObjectMeta.Name)...) } - return errors + return errors, nil } @@ -165,8 +166,12 @@ func (r *OpenStackDataPlaneNodeSet) ValidateUpdate(old runtime.Object) (admissio return nil, apierrors.NewInternalError( fmt.Errorf("expected a OpenStackDataPlaneNodeSet object, but got %T", oldNodeSet)) } + errors, err := r.validateNodes() + if err != nil { + return nil, err + } - errors := r.Spec.ValidateUpdate(&oldNodeSet.Spec) + errors = append(errors, r.Spec.ValidateUpdate(&oldNodeSet.Spec)...) if errors != nil { openstackdataplanenodesetlog.Info("validation failed", "name", r.Name)