diff --git a/Makefile b/Makefile index 426315a9d..91b817f7f 100644 --- a/Makefile +++ b/Makefile @@ -155,6 +155,10 @@ test: manifests generate fmt vet envtest ginkgo ## Run tests. OPERATOR_TEMPLATES="$(PWD)/templates" \ $(GINKGO) --trace --cover --coverpkg=../../internal/...,../../api/nova/v1beta1,../../api/placement/v1beta1 --coverprofile cover.out --covermode=atomic --randomize-all ${PROC_CMD} $(GINKGO_ARGS) ./test/... +.PHONY: gotest-unit +gotest-unit: ## Run unit tests under test/unit/ (also run via ginkgo in the test target). + go test -v ./test/unit/... + ##@ Build .PHONY: build diff --git a/api/placement/v1beta1/api_types.go b/api/placement/v1beta1/api_types.go index bdaf922a0..41bc863ec 100644 --- a/api/placement/v1beta1/api_types.go +++ b/api/placement/v1beta1/api_types.go @@ -247,11 +247,26 @@ func SetupDefaults() { SetupPlacementAPIDefaults(placementDefaults) } -// GetSecret returns the value of the Nova.Spec.Secret +// GetSecret returns the value of the PlacementAPI.Spec.Secret func (instance PlacementAPI) GetSecret() string { return instance.Spec.Secret } +// GetSpecTopologyRef returns the TopologyRef from the Spec +func (instance *PlacementAPI) GetSpecTopologyRef() *topologyv1.TopoRef { + return instance.Spec.TopologyRef +} + +// GetLastAppliedTopology returns the LastAppliedTopology from the Status +func (instance *PlacementAPI) GetLastAppliedTopology() *topologyv1.TopoRef { + return instance.Status.LastAppliedTopology +} + +// SetLastAppliedTopology sets the LastAppliedTopology value in the Status +func (instance *PlacementAPI) SetLastAppliedTopology(topologyRef *topologyv1.TopoRef) { + instance.Status.LastAppliedTopology = topologyRef +} + // ValidateTopology - func (instance *PlacementAPISpecCore) ValidateTopology( basePath *field.Path, diff --git a/cmd/main.go b/cmd/main.go index f2a2af84e..45cc8b7cb 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -38,6 +38,8 @@ import ( metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server" "sigs.k8s.io/controller-runtime/pkg/webhook" + placementv1 "github.com/openstack-k8s-operators/nova-operator/api/placement/v1beta1" + internalcommon "github.com/openstack-k8s-operators/nova-operator/internal/common" novacontroller "github.com/openstack-k8s-operators/nova-operator/internal/controller/nova" placementcontroller "github.com/openstack-k8s-operators/nova-operator/internal/controller/placement" webhookv1beta1 "github.com/openstack-k8s-operators/nova-operator/internal/webhook/nova/v1beta1" @@ -51,7 +53,6 @@ import ( "github.com/openstack-k8s-operators/lib-common/modules/common/operator" mariadbv1 "github.com/openstack-k8s-operators/mariadb-operator/api/v1beta1" novav1 "github.com/openstack-k8s-operators/nova-operator/api/nova/v1beta1" - placementv1 "github.com/openstack-k8s-operators/nova-operator/api/placement/v1beta1" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" "k8s.io/client-go/kubernetes" @@ -265,9 +266,7 @@ func main() { // Setup PlacementAPI controller if err = (&placementcontroller.PlacementAPIReconciler{ - Client: mgr.GetClient(), - Scheme: mgr.GetScheme(), - Kclient: kclient, + ReconcilerBase: internalcommon.NewReconcilerBase(mgr, kclient), }).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "PlacementAPI") os.Exit(1) diff --git a/go.mod b/go.mod index e30280c95..fb4325722 100644 --- a/go.mod +++ b/go.mod @@ -102,6 +102,7 @@ require ( google.golang.org/genproto/googleapis/rpc v0.0.0-20250115164207-1a7da9e5054f // indirect google.golang.org/grpc v1.71.1 // indirect google.golang.org/protobuf v1.36.7 // indirect + gopkg.in/evanphx/json-patch.v4 v4.12.0 // indirect gopkg.in/inf.v0 v0.9.1 // indirect gopkg.in/yaml.v2 v2.4.0 // indirect k8s.io/apiextensions-apiserver v0.33.2 // indirect diff --git a/internal/common/config.go b/internal/common/config.go new file mode 100644 index 000000000..88253a2ca --- /dev/null +++ b/internal/common/config.go @@ -0,0 +1,102 @@ +/* +Copyright 2026. + +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 common //nolint:revive // common is the established package name for multi-group shared code + +import ( + "context" + + "github.com/openstack-k8s-operators/lib-common/modules/common/env" + helper "github.com/openstack-k8s-operators/lib-common/modules/common/helper" + "github.com/openstack-k8s-operators/lib-common/modules/common/secret" + util "github.com/openstack-k8s-operators/lib-common/modules/common/util" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +// GenerateConfigs helper function to generate config maps +func GenerateConfigs( + ctx context.Context, h *helper.Helper, + instance client.Object, configName string, envVars *map[string]env.Setter, + templateParameters map[string]any, + extraData map[string]string, cmLabels map[string]string, + additionalTemplates map[string]string, + commonTemplates []string, + templateDir string, +) error { + return generateConfigsGeneric(ctx, h, instance, configName, envVars, templateParameters, extraData, + cmLabels, additionalTemplates, commonTemplates, templateDir, false) +} + +// GenerateConfigsWithScripts helper function to generate config maps +// for service configs and scripts +func GenerateConfigsWithScripts( + ctx context.Context, h *helper.Helper, + instance client.Object, envVars *map[string]env.Setter, + templateParameters map[string]any, + extraData map[string]string, cmLabels map[string]string, + additionalTemplates map[string]string, + commonTemplates []string, + templateDir string, +) error { + return generateConfigsGeneric(ctx, h, instance, GetServiceConfigSecretName(instance.GetName()), + envVars, templateParameters, extraData, + cmLabels, additionalTemplates, commonTemplates, templateDir, true) +} + +// generateConfigsGeneric helper function to generate config maps +func generateConfigsGeneric( + ctx context.Context, h *helper.Helper, + instance client.Object, configName string, envVars *map[string]env.Setter, + templateParameters map[string]any, + extraData map[string]string, cmLabels map[string]string, + additionalTemplates map[string]string, + commonTemplates []string, + templateDir string, + withScripts bool, +) error { + if templateDir == "" { + return ErrTemplateDirUnset + } + + cms := []util.Template{ + { + Name: configName, + Namespace: instance.GetNamespace(), + Type: util.TemplateTypeConfig, + InstanceType: instance.GetObjectKind().GroupVersionKind().Kind, + MultiTemplateDir: templateDir, + ConfigOptions: templateParameters, + Labels: cmLabels, + CustomData: extraData, + Annotations: map[string]string{}, + AdditionalTemplate: additionalTemplates, + CommonTemplates: commonTemplates, + }, + } + if withScripts { + cms = append(cms, util.Template{ + Name: GetScriptSecretName(instance.GetName()), + Namespace: instance.GetNamespace(), + Type: util.TemplateTypeScripts, + InstanceType: instance.GetObjectKind().GroupVersionKind().Kind, + MultiTemplateDir: templateDir, + AdditionalTemplate: map[string]string{}, + Annotations: map[string]string{}, + Labels: cmLabels, + }) + } + return secret.EnsureSecrets(ctx, h, instance, cms, envVars) +} diff --git a/internal/common/errors.go b/internal/common/errors.go new file mode 100644 index 000000000..e8e205026 --- /dev/null +++ b/internal/common/errors.go @@ -0,0 +1,30 @@ +/* +Copyright 2026. + +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 common //nolint:revive // common is the established package name for multi-group shared code + +import "errors" + +// Static errors shared across operator services. +var ( + // ErrACSecretMissingKeys indicates that the ApplicationCredential secret is missing required keys. + ErrACSecretMissingKeys = errors.New("ApplicationCredential secret missing required keys") + // ErrTemplateDirUnset indicates that no template directory was provided. + ErrTemplateDirUnset = errors.New("templateDir must be set") + + // ErrUnexpectedObjectType is returned when a webhook receives an unexpected object type. + ErrUnexpectedObjectType = errors.New("unexpected object type") +) diff --git a/internal/common/kolla.go b/internal/common/kolla.go new file mode 100644 index 000000000..3524a0739 --- /dev/null +++ b/internal/common/kolla.go @@ -0,0 +1,37 @@ +/* +Copyright 2026. + +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 common provides shared helpers for nova, placement, and cyborg controllers. +package common //nolint:revive // common is the established package name for multi-group shared code + +import "fmt" + +const ( + // ServiceCommand - the command to start the service binary in the kolla container + ServiceCommand = "/usr/local/bin/kolla_start" +) + +// GetScriptSecretName returns the name of the Secret used for the +// db sync scripts +func GetScriptSecretName(crName string) string { + return fmt.Sprintf("%s-scripts", crName) +} + +// GetServiceConfigSecretName returns the name of the Secret used to +// store the service configuration files +func GetServiceConfigSecretName(crName string) string { + return fmt.Sprintf("%s-config-data", crName) +} diff --git a/internal/common/network.go b/internal/common/network.go new file mode 100644 index 000000000..d72d6dcd1 --- /dev/null +++ b/internal/common/network.go @@ -0,0 +1,82 @@ +/* +Copyright 2026. + +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 common //nolint:revive // common is the established package name for multi-group shared code + +import ( + "context" + "fmt" + "time" + + networkv1 "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1" + condition "github.com/openstack-k8s-operators/lib-common/modules/common/condition" + helper "github.com/openstack-k8s-operators/lib-common/modules/common/helper" + nad "github.com/openstack-k8s-operators/lib-common/modules/common/networkattachment" + k8s_errors "k8s.io/apimachinery/pkg/api/errors" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/log" +) + +// EnsureNetworkAttachments - checks the requested network attachments exists and +// returns the annotation to be set on the deployment objects. +func EnsureNetworkAttachments( + ctx context.Context, + h *helper.Helper, + networkAttachments []string, + conditionUpdater ConditionUpdater, + requeueTimeout time.Duration, +) (map[string]string, ctrl.Result, error) { + var nadAnnotations map[string]string + + // networks to attach to + nadList := []networkv1.NetworkAttachmentDefinition{} + for _, netAtt := range networkAttachments { + nadObject, err := nad.GetNADWithName(ctx, h, netAtt, h.GetBeforeObject().GetNamespace()) + if err != nil { + if k8s_errors.IsNotFound(err) { + // Since the net-attach-def CR should have been manually created by the user and referenced in the spec, + // we treat this as a warning because it means that the service will not be able to start. + log.FromContext(ctx).Info(fmt.Sprintf("network-attachment-definition %s not found", netAtt)) + conditionUpdater.Set(condition.FalseCondition( + condition.NetworkAttachmentsReadyCondition, + condition.ErrorReason, + condition.SeverityWarning, + condition.NetworkAttachmentsReadyWaitingMessage, + netAtt)) + return nadAnnotations, ctrl.Result{RequeueAfter: requeueTimeout}, nil + } + conditionUpdater.Set(condition.FalseCondition( + condition.NetworkAttachmentsReadyCondition, + condition.ErrorReason, + condition.SeverityWarning, + condition.NetworkAttachmentsErrorMessage, + err.Error())) + return nadAnnotations, ctrl.Result{}, err + } + + if nadObject != nil { + nadList = append(nadList, *nadObject) + } + } + + nadAnnotations, err := nad.EnsureNetworksAnnotation(nadList) + if err != nil { + return nadAnnotations, ctrl.Result{}, fmt.Errorf("failed create network annotation from %s: %w", + networkAttachments, err) + } + + return nadAnnotations, ctrl.Result{}, nil +} diff --git a/internal/common/reconciler.go b/internal/common/reconciler.go new file mode 100644 index 000000000..09b077779 --- /dev/null +++ b/internal/common/reconciler.go @@ -0,0 +1,94 @@ +/* +Copyright 2022. + +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 common //nolint:revive // common is the established package name for multi-group shared code + +import ( + "time" + + "github.com/go-logr/logr" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/kubernetes" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +// ReconcilerBase provides a common set of clients scheme and loggers for all reconcilers. +type ReconcilerBase struct { + Client client.Client + Kclient kubernetes.Interface + Scheme *runtime.Scheme + RequeueTimeout time.Duration +} + +// Manageable all types that conform to this interface can be setup with a controller-runtime manager. +type Manageable interface { + SetupWithManager(mgr ctrl.Manager) error +} + +// Reconciler represents a generic interface for all Reconciler objects in nova +type Reconciler interface { + Manageable + SetRequeueTimeout(timeout time.Duration) +} + +// NewReconcilerBase constructs a ReconcilerBase given a manager and Kclient. +func NewReconcilerBase( + mgr ctrl.Manager, kclient kubernetes.Interface, +) ReconcilerBase { + return ReconcilerBase{ + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + Kclient: kclient, + RequeueTimeout: time.Duration(5) * time.Second, + } +} + +// SetRequeueTimeout overrides the default RequeueTimeout of the Reconciler +func (r *ReconcilerBase) SetRequeueTimeout(timeout time.Duration) { + r.RequeueTimeout = timeout +} + +// Reconcilers holds all the Reconciler objects of the nova-operator to +// allow generic management of them. +type Reconcilers struct { + reconcilers map[string]Reconciler +} + +// NewReconcilers constructs a Reconcilers registry from the given controllers. +func NewReconcilers(reconcilers map[string]Reconciler) *Reconcilers { + return &Reconcilers{ + reconcilers: reconcilers, + } +} + +// Setup starts the reconcilers by connecting them to the Manager +func (r *Reconcilers) Setup(mgr ctrl.Manager, setupLog logr.Logger) error { + for name, controller := range r.reconcilers { + if err := controller.SetupWithManager(mgr); err != nil { + setupLog.Error(err, "unable to create controller", "controller", name) + return err + } + } + return nil +} + +// OverrideRequeueTimeout overrides the default RequeueTimeout of our reconcilers +func (r *Reconcilers) OverrideRequeueTimeout(timeout time.Duration) { + for _, reconciler := range r.reconcilers { + reconciler.SetRequeueTimeout(timeout) + } +} diff --git a/internal/common/secret.go b/internal/common/secret.go new file mode 100644 index 000000000..69ee284c7 --- /dev/null +++ b/internal/common/secret.go @@ -0,0 +1,101 @@ +/* +Copyright 2026. + +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 common //nolint:revive // common is the established package name for multi-group shared code + +import ( + "context" + "fmt" + "time" + + condition "github.com/openstack-k8s-operators/lib-common/modules/common/condition" + util "github.com/openstack-k8s-operators/lib-common/modules/common/util" + corev1 "k8s.io/api/core/v1" + k8s_errors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/types" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/log" +) + +// EnsureSecret ensures that the Secret object exists and the expected fields +// are in the Secret. It returns a hash of the values of the expected fields. +func EnsureSecret( + ctx context.Context, + secretName types.NamespacedName, + expectedFields []string, + reader client.Reader, + conditionUpdater ConditionUpdater, + requeueTimeout time.Duration, +) (string, ctrl.Result, corev1.Secret, error) { + secret := &corev1.Secret{} + err := reader.Get(ctx, secretName, secret) + if err != nil { + if k8s_errors.IsNotFound(err) { + // Since the service secret should have been manually created by the user and referenced in the spec, + // we treat this as a warning because it means that the service will not be able to start. + log.FromContext(ctx).Info(fmt.Sprintf("secret %s not found", secretName)) + conditionUpdater.Set(condition.FalseCondition( + condition.InputReadyCondition, + condition.ErrorReason, + condition.SeverityWarning, + "Input data resources missing: %s", "secret/"+secretName.Name)) + return "", + ctrl.Result{RequeueAfter: requeueTimeout}, + *secret, + nil + } + conditionUpdater.Set(condition.FalseCondition( + condition.InputReadyCondition, + condition.ErrorReason, + condition.SeverityWarning, + condition.InputReadyErrorMessage, + err.Error())) + return "", ctrl.Result{}, *secret, err + } + + // collect the secret values the caller expects to exist + values := [][]byte{} + for _, field := range expectedFields { + val, ok := secret.Data[field] + if !ok { + err := fmt.Errorf("%w: '%s' not found in secret/%s", util.ErrFieldNotFound, field, secretName.Name) + conditionUpdater.Set(condition.FalseCondition( + condition.InputReadyCondition, + condition.ErrorReason, + condition.SeverityWarning, + condition.InputReadyErrorMessage, + err.Error())) + return "", ctrl.Result{}, *secret, err + } + values = append(values, val) + } + + // TODO(gibi): Do we need to watch the Secret for changes? + + hash, err := util.ObjectHash(values) + if err != nil { + conditionUpdater.Set(condition.FalseCondition( + condition.InputReadyCondition, + condition.ErrorReason, + condition.SeverityWarning, + condition.InputReadyErrorMessage, + err.Error())) + return "", ctrl.Result{}, *secret, err + } + + return hash, ctrl.Result{}, *secret, nil +} diff --git a/internal/common/topology.go b/internal/common/topology.go new file mode 100644 index 000000000..8581f559c --- /dev/null +++ b/internal/common/topology.go @@ -0,0 +1,77 @@ +/* +Copyright 2026. + +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 common //nolint:revive // common is the established package name for multi-group shared code + +import ( + "context" + "fmt" + + topologyv1 "github.com/openstack-k8s-operators/infra-operator/apis/topology/v1beta1" + condition "github.com/openstack-k8s-operators/lib-common/modules/common/condition" + helper "github.com/openstack-k8s-operators/lib-common/modules/common/helper" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// topologyHandler provides topology reference state for a reconciled object. +type topologyHandler interface { + GetSpecTopologyRef() *topologyv1.TopoRef + GetLastAppliedTopology() *topologyv1.TopoRef + SetLastAppliedTopology(t *topologyv1.TopoRef) +} + +// EnsureTopology - when a Topology CR is referenced, remove the +// finalizer from a previous referenced Topology (if any), and retrieve the +// newly referenced topology object +func EnsureTopology( + ctx context.Context, + helper *helper.Helper, + instance topologyHandler, + finalizer string, + conditionUpdater ConditionUpdater, + defaultLabelSelector metav1.LabelSelector, +) (*topologyv1.Topology, error) { + topology, err := topologyv1.EnsureServiceTopology( + ctx, + helper, + instance.GetSpecTopologyRef(), + instance.GetLastAppliedTopology(), + finalizer, + defaultLabelSelector, + ) + if err != nil { + conditionUpdater.Set(condition.FalseCondition( + condition.TopologyReadyCondition, + condition.ErrorReason, + condition.SeverityWarning, + condition.TopologyReadyErrorMessage, + err.Error())) + return nil, fmt.Errorf("waiting for Topology requirements: %w", err) + } + // update the Status with the last retrieved Topology (or set it to nil) + tr := instance.GetSpecTopologyRef() + instance.SetLastAppliedTopology(tr) + // update the Topology condition only when a Topology is referenced and has + // been retrieved (err == nil) + if tr != nil { + // update the TopologyRef associated condition + conditionUpdater.MarkTrue( + condition.TopologyReadyCondition, + condition.TopologyReadyMessage, + ) + } + return topology, nil +} diff --git a/internal/common/util.go b/internal/common/util.go new file mode 100644 index 000000000..99146c616 --- /dev/null +++ b/internal/common/util.go @@ -0,0 +1,27 @@ +/* +Copyright 2026. + +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 common //nolint:revive // common is the established package name for multi-group shared code + +import ( + condition "github.com/openstack-k8s-operators/lib-common/modules/common/condition" +) + +// ConditionUpdater updates conditions on a reconciled object. +type ConditionUpdater interface { + Set(c *condition.Condition) + MarkTrue(t condition.Type, messageFormat string, messageArgs ...any) +} diff --git a/internal/common/watch.go b/internal/common/watch.go new file mode 100644 index 000000000..6040d6a6f --- /dev/null +++ b/internal/common/watch.go @@ -0,0 +1,137 @@ +/* +Copyright 2026. + +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 common //nolint:revive // common is the established package name for multi-group shared code + +import ( + "context" + "fmt" + + "github.com/go-logr/logr" + "github.com/openstack-k8s-operators/lib-common/modules/common" + "github.com/openstack-k8s-operators/lib-common/modules/common/util" + "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/apimachinery/pkg/fields" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/reconcile" +) + +// FindObjectsForSrcByField returns reconcile requests for CRs in src's namespace +// that reference src via one of the indexed watchFields. +func FindObjectsForSrcByField( + ctx context.Context, + log logr.Logger, + reader client.Reader, + src client.Object, + watchFields []string, + newList func() client.ObjectList, +) []reconcile.Request { + var requests []reconcile.Request + + for _, field := range watchFields { + crList := newList() + listOps := &client.ListOptions{ + FieldSelector: fields.OneTermEqualSelector(field, src.GetName()), + Namespace: src.GetNamespace(), + } + err := reader.List(ctx, crList, listOps) + if err != nil { + log.Error(err, fmt.Sprintf("listing %s for field: %s - %s", crList.GetObjectKind().GroupVersionKind().Kind, field, src.GetNamespace())) + return requests + } + + items, err := meta.ExtractList(crList) + if err != nil { + log.Error(err, fmt.Sprintf("extracting items from %s", crList.GetObjectKind().GroupVersionKind().Kind)) + return requests + } + requests = appendRequestsForObjects(log, src, requests, items) + } + + return requests +} + +// FindObjectsForSrcInNamespace returns reconcile requests for all CRs in src's namespace. +func FindObjectsForSrcInNamespace( + ctx context.Context, + log logr.Logger, + reader client.Reader, + src client.Object, + newList func() client.ObjectList, +) []reconcile.Request { + crList := newList() + listOps := &client.ListOptions{ + Namespace: src.GetNamespace(), + } + err := reader.List(ctx, crList, listOps) + if err != nil { + log.Error(err, fmt.Sprintf("listing %s for namespace: %s", crList.GetObjectKind().GroupVersionKind().Kind, src.GetNamespace())) + return nil + } + + items, err := meta.ExtractList(crList) + if err != nil { + log.Error(err, fmt.Sprintf("extracting items from %s", crList.GetObjectKind().GroupVersionKind().Kind)) + return nil + } + return appendRequestsForObjects(log, src, nil, items) +} + +// FindObjectsWithAppSelectorLabelInNamespace returns reconcile requests for all CRs +// in src's namespace when src carries an AppSelector label matching allowedServices. +func FindObjectsWithAppSelectorLabelInNamespace( + ctx context.Context, + log logr.Logger, + reader client.Reader, + src client.Object, + allowedServices []string, + newList func() client.ObjectList, +) []reconcile.Request { + if svc, ok := src.GetLabels()[common.AppSelector]; ok && util.StringInSlice(svc, allowedServices) { + return FindObjectsForSrcInNamespace(ctx, log, reader, src, newList) + } + + return nil +} + +func appendRequestsForObjects( + log logr.Logger, + src client.Object, + requests []reconcile.Request, + items []runtime.Object, +) []reconcile.Request { + for _, item := range items { + accessor, err := meta.Accessor(item) + if err != nil { + log.Error(err, "extracting object metadata") + continue + } + log.Info(fmt.Sprintf("input source %s changed, reconcile: %s - %s", src.GetName(), accessor.GetName(), accessor.GetNamespace())) + + requests = append(requests, + reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: accessor.GetName(), + Namespace: accessor.GetNamespace(), + }, + }, + ) + } + + return requests +} diff --git a/internal/controller/nova/common.go b/internal/controller/nova/common.go index a533d165c..9af74e08c 100644 --- a/internal/controller/nova/common.go +++ b/internal/controller/nova/common.go @@ -19,9 +19,7 @@ package controller import ( "context" - "errors" "fmt" - "maps" "net/url" "sort" "strconv" @@ -30,10 +28,7 @@ import ( "github.com/go-logr/logr" appsv1 "k8s.io/api/apps/v1" - corev1 "k8s.io/api/core/v1" k8s_errors "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/kubernetes" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -41,21 +36,16 @@ import ( memcachedv1 "github.com/openstack-k8s-operators/infra-operator/apis/memcached/v1beta1" novav1 "github.com/openstack-k8s-operators/nova-operator/api/nova/v1beta1" - "github.com/openstack-k8s-operators/nova-operator/internal/nova" + internalcommon "github.com/openstack-k8s-operators/nova-operator/internal/common" gophercloud "github.com/gophercloud/gophercloud/v2" "github.com/gophercloud/gophercloud/v2/openstack/compute/v2/services" - networkv1 "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1" - topologyv1 "github.com/openstack-k8s-operators/infra-operator/apis/topology/v1beta1" "github.com/openstack-k8s-operators/lib-common/modules/common/condition" - "github.com/openstack-k8s-operators/lib-common/modules/common/env" helper "github.com/openstack-k8s-operators/lib-common/modules/common/helper" - nad "github.com/openstack-k8s-operators/lib-common/modules/common/networkattachment" "github.com/openstack-k8s-operators/lib-common/modules/common/secret" "github.com/openstack-k8s-operators/lib-common/modules/common/tls" util "github.com/openstack-k8s-operators/lib-common/modules/common/util" "github.com/openstack-k8s-operators/lib-common/modules/openstack" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) const ( @@ -154,66 +144,8 @@ var ( endpointNeutron, endpointPlacement, } - - // ErrACSecretMissingKeys indicates that the ApplicationCredential secret is missing required keys - ErrACSecretMissingKeys = errors.New("ApplicationCredential secret missing required keys") - // ErrTemplateDirUnset indicates that no template directory was provided. - ErrTemplateDirUnset = errors.New("templateDir must be set") ) -type conditionsGetter interface { - GetConditions() condition.Conditions -} - -type topologyHandler interface { - GetSpecTopologyRef() *topologyv1.TopoRef - GetLastAppliedTopology() *topologyv1.TopoRef - SetLastAppliedTopology(t *topologyv1.TopoRef) -} - -// ensureTopology - when a Topology CR is referenced, remove the -// finalizer from a previous referenced Topology (if any), and retrieve the -// newly referenced topology object -func ensureTopology( - ctx context.Context, - helper *helper.Helper, - instance topologyHandler, - finalizer string, - conditionUpdater conditionUpdater, - defaultLabelSelector metav1.LabelSelector, -) (*topologyv1.Topology, error) { - topology, err := topologyv1.EnsureServiceTopology( - ctx, - helper, - instance.GetSpecTopologyRef(), - instance.GetLastAppliedTopology(), - finalizer, - defaultLabelSelector, - ) - if err != nil { - conditionUpdater.Set(condition.FalseCondition( - condition.TopologyReadyCondition, - condition.ErrorReason, - condition.SeverityWarning, - condition.TopologyReadyErrorMessage, - err.Error())) - return nil, fmt.Errorf("waiting for Topology requirements: %w", err) - } - // update the Status with the last retrieved Topology (or set it to nil) - tr := instance.GetSpecTopologyRef() - instance.SetLastAppliedTopology(tr) - // update the Topology condition only when a Topology is referenced and has - // been retrieved (err == nil) - if tr != nil { - // update the TopologyRef associated condition - conditionUpdater.MarkTrue( - condition.TopologyReadyCondition, - condition.TopologyReadyMessage, - ) - } - return topology, nil -} - func cleanNovaServiceFromNovaDb( ctx context.Context, computeClient *gophercloud.ServiceClient, @@ -269,162 +201,9 @@ func cleanNovaServiceFromNovaDb( return err } -func allSubConditionIsTrue(conditionsGetter conditionsGetter) bool { - // It assumes that all of our conditions report success via the True status - for _, c := range conditionsGetter.GetConditions() { - if c.Type == condition.ReadyCondition { - continue - } - if c.Status != corev1.ConditionTrue { - return false - } - } - return true -} - -type conditionUpdater interface { - Set(c *condition.Condition) - MarkTrue(t condition.Type, messageFormat string, messageArgs ...any) -} - -func ensureSecret( - ctx context.Context, - secretName types.NamespacedName, - expectedFields []string, - reader client.Reader, - conditionUpdater conditionUpdater, - requeueTimeout time.Duration, -) (string, ctrl.Result, corev1.Secret, error) { - secret := &corev1.Secret{} - err := reader.Get(ctx, secretName, secret) - if err != nil { - if k8s_errors.IsNotFound(err) { - // This is only currently used to get the password secret provided by the user in the spec. - // Therefore, since the secret should have been manually created by the user and referenced - // in the spec, we treat this as a warning because it means that the service will not be - // able to start. - log.FromContext(ctx).Info(fmt.Sprintf("secret %s not found", secretName)) - conditionUpdater.Set(condition.FalseCondition( - condition.InputReadyCondition, - condition.ErrorReason, - condition.SeverityWarning, - novav1.InputReadyWaitingMessage, "secret/"+secretName.Name)) - return "", - ctrl.Result{RequeueAfter: requeueTimeout}, - *secret, - nil - } - conditionUpdater.Set(condition.FalseCondition( - condition.InputReadyCondition, - condition.ErrorReason, - condition.SeverityWarning, - condition.InputReadyErrorMessage, - err.Error())) - return "", ctrl.Result{}, *secret, err - } - - // collect the secret values the caller expects to exist - values := [][]byte{} - for _, field := range expectedFields { - val, ok := secret.Data[field] - if !ok { - err := fmt.Errorf("%w: '%s' not found in secret/%s", util.ErrFieldNotFound, field, secretName.Name) - conditionUpdater.Set(condition.FalseCondition( - condition.InputReadyCondition, - condition.ErrorReason, - condition.SeverityWarning, - condition.InputReadyErrorMessage, - err.Error())) - return "", ctrl.Result{}, *secret, err - } - values = append(values, val) - } - - // TODO(gibi): Do we need to watch the Secret for changes? - - hash, err := util.ObjectHash(values) - if err != nil { - conditionUpdater.Set(condition.FalseCondition( - condition.InputReadyCondition, - condition.ErrorReason, - condition.SeverityWarning, - condition.InputReadyErrorMessage, - err.Error())) - return "", ctrl.Result{}, *secret, err - } - - return hash, ctrl.Result{}, *secret, nil -} - -// ensureNetworkAttachments - checks the requested network attachments exists and -// returns the annotation to be set on the deployment objects. -func ensureNetworkAttachments( - ctx context.Context, - h *helper.Helper, - networkAttachments []string, - conditionUpdater conditionUpdater, - requeueTimeout time.Duration, -) (map[string]string, ctrl.Result, error) { - var nadAnnotations map[string]string - var err error - - // networks to attach to - nadList := []networkv1.NetworkAttachmentDefinition{} - for _, netAtt := range networkAttachments { - nad, err := nad.GetNADWithName(ctx, h, netAtt, h.GetBeforeObject().GetNamespace()) - if err != nil { - if k8s_errors.IsNotFound(err) { - // Since the net-attach-def CR should have been manually created by the user and referenced in the spec, - // we treat this as a warning because it means that the service will not be able to start. - log.FromContext(ctx).Info(fmt.Sprintf("network-attachment-definition %s not found", netAtt)) - conditionUpdater.Set(condition.FalseCondition( - condition.NetworkAttachmentsReadyCondition, - condition.ErrorReason, - condition.SeverityWarning, - condition.NetworkAttachmentsReadyWaitingMessage, - netAtt)) - return nadAnnotations, ctrl.Result{RequeueAfter: requeueTimeout}, nil - } - conditionUpdater.Set(condition.FalseCondition( - condition.NetworkAttachmentsReadyCondition, - condition.ErrorReason, - condition.SeverityWarning, - condition.NetworkAttachmentsErrorMessage, - err.Error())) - return nadAnnotations, ctrl.Result{}, err - } - - if nad != nil { - nadList = append(nadList, *nad) - } - } - - nadAnnotations, err = nad.EnsureNetworksAnnotation(nadList) - if err != nil { - return nadAnnotations, ctrl.Result{}, fmt.Errorf("failed create network annotation from %s: %w", - networkAttachments, err) - } - - return nadAnnotations, ctrl.Result{}, nil -} - // ReconcilerBase provides a common set of clients scheme and loggers for all reconcilers. type ReconcilerBase struct { - Client client.Client - Kclient kubernetes.Interface - Scheme *runtime.Scheme - RequeueTimeout time.Duration -} - -// Manageable all types that conform to this interface can be setup with a controller-runtime manager. -type Manageable interface { - SetupWithManager(mgr ctrl.Manager) error -} - -// Reconciler represents a generic interface for all Reconciler objects in nova -type Reconciler interface { - Manageable - SetRequeueTimeout(timeout time.Duration) + internalcommon.ReconcilerBase } // NewReconcilerBase constructs a ReconcilerBase given a manager and Kclient. @@ -432,157 +211,46 @@ func NewReconcilerBase( mgr ctrl.Manager, kclient kubernetes.Interface, ) ReconcilerBase { return ReconcilerBase{ - Client: mgr.GetClient(), - Scheme: mgr.GetScheme(), - Kclient: kclient, - RequeueTimeout: time.Duration(5) * time.Second, + ReconcilerBase: internalcommon.NewReconcilerBase(mgr, kclient), } } -// SetRequeueTimeout overrides the default RequeueTimeout of the Reconciler -func (r *ReconcilerBase) SetRequeueTimeout(timeout time.Duration) { - r.RequeueTimeout = timeout -} - -// Reconcilers holds all the Reconciler objects of the nova-operator to -// allow generic management of them. -type Reconcilers struct { - reconcilers map[string]Reconciler -} - // NewReconcilers constructs all nova Reconciler objects -func NewReconcilers(mgr ctrl.Manager, kclient *kubernetes.Clientset) *Reconcilers { - return &Reconcilers{ - reconcilers: map[string]Reconciler{ - "Nova": &NovaReconciler{ - ReconcilerBase: NewReconcilerBase(mgr, kclient), - }, - "NovaCell": &NovaCellReconciler{ - ReconcilerBase: NewReconcilerBase(mgr, kclient), - }, - "NovaAPI": &NovaAPIReconciler{ - ReconcilerBase: NewReconcilerBase(mgr, kclient), - }, - "NovaScheduler": &NovaSchedulerReconciler{ - ReconcilerBase: NewReconcilerBase(mgr, kclient), - }, - "NovaConductor": &NovaConductorReconciler{ - ReconcilerBase: NewReconcilerBase(mgr, kclient), - }, - "NovaMetadata": &NovaMetadataReconciler{ - ReconcilerBase: NewReconcilerBase(mgr, kclient), - }, - "NovaNoVNCProxy": &NovaNoVNCProxyReconciler{ - ReconcilerBase: NewReconcilerBase(mgr, kclient), - }, - "NovaCompute": &NovaComputeReconciler{ - ReconcilerBase: NewReconcilerBase(mgr, kclient), - }, +func NewReconcilers(mgr ctrl.Manager, kclient *kubernetes.Clientset) *internalcommon.Reconcilers { + return internalcommon.NewReconcilers(map[string]internalcommon.Reconciler{ + "Nova": &NovaReconciler{ + ReconcilerBase: NewReconcilerBase(mgr, kclient), }, - } -} - -// Setup starts the reconcilers by connecting them to the Manager -func (r *Reconcilers) Setup(mgr ctrl.Manager, setupLog logr.Logger) error { - var err error - for name, controller := range r.reconcilers { - if err = controller.SetupWithManager(mgr); err != nil { - setupLog.Error(err, "unable to create controller", "controller", name) - return err - } - } - return nil -} - -// OverrideRequeueTimeout overrides the default RequeueTimeout of our reconcilers -func (r *Reconcilers) OverrideRequeueTimeout(timeout time.Duration) { - for _, reconciler := range r.reconcilers { - reconciler.SetRequeueTimeout(timeout) - } + "NovaCell": &NovaCellReconciler{ + ReconcilerBase: NewReconcilerBase(mgr, kclient), + }, + "NovaAPI": &NovaAPIReconciler{ + ReconcilerBase: NewReconcilerBase(mgr, kclient), + }, + "NovaScheduler": &NovaSchedulerReconciler{ + ReconcilerBase: NewReconcilerBase(mgr, kclient), + }, + "NovaConductor": &NovaConductorReconciler{ + ReconcilerBase: NewReconcilerBase(mgr, kclient), + }, + "NovaMetadata": &NovaMetadataReconciler{ + ReconcilerBase: NewReconcilerBase(mgr, kclient), + }, + "NovaNoVNCProxy": &NovaNoVNCProxyReconciler{ + ReconcilerBase: NewReconcilerBase(mgr, kclient), + }, + "NovaCompute": &NovaComputeReconciler{ + ReconcilerBase: NewReconcilerBase(mgr, kclient), + }, + }) } -// generateConfigsGeneric helper function to generate config maps -func (r *ReconcilerBase) generateConfigsGeneric( - ctx context.Context, h *helper.Helper, - instance client.Object, configName string, envVars *map[string]env.Setter, - templateParameters map[string]any, - extraData map[string]string, cmLabels map[string]string, - additionalTemplates map[string]string, - commonTemplates []string, - templateDir string, - withScripts bool, -) error { - extraTemplates := map[string]string{ +// novaAdditionalTemplates returns the default extra config templates for nova services. +func novaAdditionalTemplates() map[string]string { + return map[string]string{ "01-nova.conf": "/nova/nova.conf", "nova-blank.conf": "/nova/nova-blank.conf", } - if templateDir == "" { - return ErrTemplateDirUnset - } - - maps.Copy(extraTemplates, additionalTemplates) - cms := []util.Template{ - { - Name: configName, - Namespace: instance.GetNamespace(), - Type: util.TemplateTypeConfig, - InstanceType: instance.GetObjectKind().GroupVersionKind().Kind, - MultiTemplateDir: templateDir, - ConfigOptions: templateParameters, - Labels: cmLabels, - CustomData: extraData, - Annotations: map[string]string{}, - AdditionalTemplate: extraTemplates, - CommonTemplates: commonTemplates, - }, - } - if withScripts { - cms = append(cms, util.Template{ - Name: nova.GetScriptSecretName(instance.GetName()), - Namespace: instance.GetNamespace(), - Type: util.TemplateTypeScripts, - InstanceType: instance.GetObjectKind().GroupVersionKind().Kind, - MultiTemplateDir: templateDir, - AdditionalTemplate: map[string]string{}, - Annotations: map[string]string{}, - Labels: cmLabels, - }) - } - return secret.EnsureSecrets(ctx, h, instance, cms, envVars) -} - -// GenerateConfigs helper function to generate config maps -func (r *ReconcilerBase) GenerateConfigs( - ctx context.Context, h *helper.Helper, - instance client.Object, configName string, envVars *map[string]env.Setter, - templateParameters map[string]any, - extraData map[string]string, cmLabels map[string]string, - additionalTemplates map[string]string, - commonTemplates []string, - templateDir string, -) error { - return r.generateConfigsGeneric( - ctx, h, instance, configName, envVars, templateParameters, extraData, - cmLabels, additionalTemplates, commonTemplates, templateDir, false, - ) -} - -// GenerateConfigsWithScripts helper function to generate config maps -// for service configs and scripts -func (r *ReconcilerBase) GenerateConfigsWithScripts( - ctx context.Context, h *helper.Helper, - instance client.Object, envVars *map[string]env.Setter, - templateParameters map[string]any, - extraData map[string]string, cmLabels map[string]string, - additionalTemplates map[string]string, - commonTemplates []string, - templateDir string, -) error { - return r.generateConfigsGeneric( - ctx, h, instance, nova.GetServiceConfigSecretName(instance.GetName()), - envVars, templateParameters, extraData, - cmLabels, additionalTemplates, commonTemplates, templateDir, true, - ) } func getNovaCellCRName(novaCRName string, cellName string) string { @@ -754,7 +422,7 @@ func ensureMemcached( h *helper.Helper, namespaceName string, memcachedName string, - conditionUpdater conditionUpdater, + conditionUpdater internalcommon.ConditionUpdater, ) (*memcachedv1.Memcached, error) { memcached, err := memcachedv1.GetMemcachedByName(ctx, h, memcachedName, namespaceName) if err != nil { diff --git a/internal/controller/nova/nova_controller.go b/internal/controller/nova/nova_controller.go index 3c0fab596..bbc2fc9de 100644 --- a/internal/controller/nova/nova_controller.go +++ b/internal/controller/nova/nova_controller.go @@ -28,7 +28,6 @@ import ( rbacv1 "k8s.io/api/rbac/v1" k8s_errors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/fields" "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/builder" @@ -55,6 +54,7 @@ import ( util "github.com/openstack-k8s-operators/lib-common/modules/common/util" novav1 "github.com/openstack-k8s-operators/nova-operator/api/nova/v1beta1" + internalcommon "github.com/openstack-k8s-operators/nova-operator/internal/common" "github.com/openstack-k8s-operators/nova-operator/internal/nova" novaapi "github.com/openstack-k8s-operators/nova-operator/internal/nova/api" @@ -157,7 +157,7 @@ func (r *NovaReconciler) Reconcile(ctx context.Context, req ctrl.Request) (resul panic(r) } // update the Ready condition based on the sub conditions - if allSubConditionIsTrue(instance.Status) { + if instance.Status.Conditions.AllSubConditionIsTrue() { instance.Status.Conditions.MarkTrue( condition.ReadyCondition, condition.ReadyMessage) } else { @@ -250,7 +250,7 @@ func (r *NovaReconciler) Reconcile(ctx context.Context, req ctrl.Request) (resul instance.Spec.PasswordSelectors.MetadataSecret, } - _, result, ospSecret, err := ensureSecret( + _, result, ospSecret, err := internalcommon.EnsureSecret( ctx, types.NamespacedName{Namespace: instance.Namespace, Name: instance.Spec.Secret}, expectedSelectors, @@ -299,7 +299,7 @@ func (r *NovaReconciler) Reconcile(ctx context.Context, req ctrl.Request) (resul condition.ErrorReason, condition.SeverityWarning, novav1.NovaApplicationCredentialSecretErrorMessage)) - return ctrl.Result{}, fmt.Errorf("%w: %s", ErrACSecretMissingKeys, instance.Spec.Auth.ApplicationCredentialSecret) + return ctrl.Result{}, fmt.Errorf("%w: %s", internalcommon.ErrACSecretMissingKeys, instance.Spec.Auth.ApplicationCredentialSecret) } } @@ -1128,11 +1128,6 @@ func (r *NovaReconciler) ensureNovaManageJobSecret( "my.cnf": cellDB.GetDatabaseClientConfig(tlsCfg), //(mschuppert) for now just get the default my.cnf } - extraTemplates := map[string]string{ - "01-nova.conf": "/nova/nova.conf", - "nova-blank.conf": "/nova/nova-blank.conf", - } - apiDatabaseAccount, apiDbSecret, err := mariadbv1.GetAccountAndSecret(ctx, h, instance.Spec.APIDatabaseAccount, instance.Namespace) if err != nil { return nil, "", "", err @@ -1197,7 +1192,7 @@ func (r *NovaReconciler) ensureNovaManageJobSecret( Labels: cmLabels, CustomData: extraData, Annotations: map[string]string{}, - AdditionalTemplate: extraTemplates, + AdditionalTemplate: novaAdditionalTemplates(), }, } @@ -2268,68 +2263,24 @@ func (r *NovaReconciler) ensureTopLevelSecret( } func (r *NovaReconciler) findObjectsForSrc(ctx context.Context, src client.Object) []reconcile.Request { - requests := []reconcile.Request{} - - Log := r.GetLogger(ctx) - - for _, field := range novaWatchFields { - crList := &novav1.NovaList{} - listOps := &client.ListOptions{ - FieldSelector: fields.OneTermEqualSelector(field, src.GetName()), - Namespace: src.GetNamespace(), - } - err := r.Client.List(ctx, crList, listOps) - if err != nil { - Log.Error(err, fmt.Sprintf("listing %s for field: %s - %s", crList.GroupVersionKind().Kind, field, src.GetNamespace())) - return requests - } - - for _, item := range crList.Items { - Log.Info(fmt.Sprintf("input source %s changed, reconcile: %s - %s", src.GetName(), item.GetName(), item.GetNamespace())) - - requests = append(requests, - reconcile.Request{ - NamespacedName: types.NamespacedName{ - Name: item.GetName(), - Namespace: item.GetNamespace(), - }, - }, - ) - } - } - - return requests + return internalcommon.FindObjectsForSrcByField( + ctx, + r.GetLogger(ctx), + r.Client, + src, + novaWatchFields, + func() client.ObjectList { return &novav1.NovaList{} }, + ) } func (r *NovaReconciler) findObjectForSrc(ctx context.Context, src client.Object) []reconcile.Request { - requests := []reconcile.Request{} - - Log := r.GetLogger(ctx) - - crList := &novav1.NovaList{} - listOps := &client.ListOptions{ - Namespace: src.GetNamespace(), - } - err := r.Client.List(ctx, crList, listOps) - if err != nil { - Log.Error(err, fmt.Sprintf("listing %s for namespace: %s", crList.GroupVersionKind().Kind, src.GetNamespace())) - return requests - } - - for _, item := range crList.Items { - Log.Info(fmt.Sprintf("input source %s changed, reconcile: %s - %s", src.GetName(), item.GetName(), item.GetNamespace())) - - requests = append(requests, - reconcile.Request{ - NamespacedName: types.NamespacedName{ - Name: item.GetName(), - Namespace: item.GetNamespace(), - }, - }, - ) - } - - return requests + return internalcommon.FindObjectsForSrcInNamespace( + ctx, + r.GetLogger(ctx), + r.Client, + src, + func() client.ObjectList { return &novav1.NovaList{} }, + ) } func (r *NovaReconciler) memcachedNamespaceMapFunc(ctx context.Context, src client.Object) []reconcile.Request { diff --git a/internal/controller/nova/novaapi_controller.go b/internal/controller/nova/novaapi_controller.go index 8d967f8c4..b8d220d87 100644 --- a/internal/controller/nova/novaapi_controller.go +++ b/internal/controller/nova/novaapi_controller.go @@ -23,7 +23,6 @@ import ( v1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/fields" "k8s.io/apimachinery/pkg/types" "k8s.io/utils/ptr" ctrl "sigs.k8s.io/controller-runtime" @@ -54,7 +53,7 @@ import ( topologyv1 "github.com/openstack-k8s-operators/infra-operator/apis/topology/v1beta1" keystonev1 "github.com/openstack-k8s-operators/keystone-operator/api/v1beta1" novav1 "github.com/openstack-k8s-operators/nova-operator/api/nova/v1beta1" - "github.com/openstack-k8s-operators/nova-operator/internal/nova" + internalcommon "github.com/openstack-k8s-operators/nova-operator/internal/common" novaapi "github.com/openstack-k8s-operators/nova-operator/internal/nova/api" k8s_errors "k8s.io/apimachinery/pkg/api/errors" @@ -143,7 +142,7 @@ func (r *NovaAPIReconciler) Reconcile(ctx context.Context, req ctrl.Request) (re panic(r) } // update the Ready condition based on the sub conditions - if allSubConditionIsTrue(instance.Status) { + if instance.Status.Conditions.AllSubConditionIsTrue() { instance.Status.Conditions.MarkTrue( condition.ReadyCondition, condition.ReadyMessage) } else { @@ -209,7 +208,7 @@ func (r *NovaAPIReconciler) Reconcile(ctx context.Context, req ctrl.Request) (re NotificationTransportURLSelector, } - secretHash, result, secret, err := ensureSecret( + secretHash, result, secret, err := internalcommon.EnsureSecret( ctx, types.NamespacedName{Namespace: instance.Namespace, Name: instance.Spec.Secret}, requiredSecretFields, @@ -331,7 +330,7 @@ func (r *NovaAPIReconciler) Reconcile(ctx context.Context, req ctrl.Request) (re instance.Status.Conditions.MarkTrue(condition.ServiceConfigReadyCondition, condition.ServiceConfigReadyMessage) - serviceAnnotations, result, err := ensureNetworkAttachments(ctx, h, instance.Spec.NetworkAttachments, &instance.Status.Conditions, r.RequeueTimeout) + serviceAnnotations, result, err := internalcommon.EnsureNetworkAttachments(ctx, h, instance.Spec.NetworkAttachments, &instance.Status.Conditions, r.RequeueTimeout) if (err != nil || result != ctrl.Result{}) { return result, err } @@ -571,12 +570,11 @@ func (r *NovaAPIReconciler) generateConfigs( instance, labels.GetGroupLabel(NovaAPILabelPrefix), map[string]string{}, ) - err = r.GenerateConfigs( - ctx, h, instance, nova.GetServiceConfigSecretName(instance.GetName()), - hashes, templateParameters, extraData, cmLabels, map[string]string{}, + return internalcommon.GenerateConfigs( + ctx, h, instance, internalcommon.GetServiceConfigSecretName(instance.GetName()), + hashes, templateParameters, extraData, cmLabels, novaAdditionalTemplates(), []string{"ssl.conf"}, "nova/api", ) - return err } func (r *NovaAPIReconciler) ensureDeployment( @@ -593,7 +591,7 @@ func (r *NovaAPIReconciler) ensureDeployment( // // Handle Topology // - topology, err := ensureTopology( + topology, err := internalcommon.EnsureTopology( ctx, h, instance, // topologyHandler @@ -928,71 +926,25 @@ func getAPIServiceLabels() map[string]string { } func (r *NovaAPIReconciler) findObjectsForSrc(ctx context.Context, src client.Object) []reconcile.Request { - requests := []reconcile.Request{} - - Log := r.GetLogger(ctx) - - for _, field := range apiWatchFields { - crList := &novav1.NovaAPIList{} - listOps := &client.ListOptions{ - FieldSelector: fields.OneTermEqualSelector(field, src.GetName()), - Namespace: src.GetNamespace(), - } - err := r.Client.List(ctx, crList, listOps) - if err != nil { - Log.Error(err, fmt.Sprintf("listing %s for field: %s - %s", crList.GroupVersionKind().Kind, field, src.GetNamespace())) - return requests - } - - for _, item := range crList.Items { - Log.Info(fmt.Sprintf("input source %s changed, reconcile: %s - %s", src.GetName(), item.GetName(), item.GetNamespace())) - - requests = append(requests, - reconcile.Request{ - NamespacedName: types.NamespacedName{ - Name: item.GetName(), - Namespace: item.GetNamespace(), - }, - }, - ) - } - } - - return requests + return internalcommon.FindObjectsForSrcByField( + ctx, + r.GetLogger(ctx), + r.Client, + src, + apiWatchFields, + func() client.ObjectList { return &novav1.NovaAPIList{} }, + ) } func (r *NovaAPIReconciler) findObjectsWithAppSelectorLabelInNamespace(ctx context.Context, src client.Object) []reconcile.Request { - requests := []reconcile.Request{} - - Log := r.GetLogger(ctx) - - // if the endpoint has the service label and its in our endpointList, reconcile the CR in the namespace - if svc, ok := src.GetLabels()[common.AppSelector]; ok && util.StringInSlice(svc, endpointList) { - crList := &novav1.NovaAPIList{} - listOps := &client.ListOptions{ - Namespace: src.GetNamespace(), - } - err := r.Client.List(ctx, crList, listOps) - if err != nil { - Log.Error(err, fmt.Sprintf("listing %s for namespace: %s", crList.GroupVersionKind().Kind, src.GetNamespace())) - return requests - } - - for _, item := range crList.Items { - Log.Info(fmt.Sprintf("input source %s changed, reconcile: %s - %s", src.GetName(), item.GetName(), item.GetNamespace())) - - requests = append(requests, - reconcile.Request{ - NamespacedName: types.NamespacedName{ - Name: item.GetName(), - Namespace: item.GetNamespace(), - }, - }, - ) - } - } - - return requests + return internalcommon.FindObjectsWithAppSelectorLabelInNamespace( + ctx, + r.GetLogger(ctx), + r.Client, + src, + endpointList, + func() client.ObjectList { return &novav1.NovaAPIList{} }, + ) } // fields to index to reconcile when change diff --git a/internal/controller/nova/novacell_controller.go b/internal/controller/nova/novacell_controller.go index 99cd060a5..de611f63d 100644 --- a/internal/controller/nova/novacell_controller.go +++ b/internal/controller/nova/novacell_controller.go @@ -24,7 +24,6 @@ import ( corev1 "k8s.io/api/core/v1" k8s_errors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/fields" "k8s.io/apimachinery/pkg/types" "k8s.io/utils/ptr" ctrl "sigs.k8s.io/controller-runtime" @@ -47,6 +46,7 @@ import ( topologyv1 "github.com/openstack-k8s-operators/infra-operator/apis/topology/v1beta1" novav1 "github.com/openstack-k8s-operators/nova-operator/api/nova/v1beta1" + internalcommon "github.com/openstack-k8s-operators/nova-operator/internal/common" ) // NovaCellReconciler reconciles a NovaCell object @@ -125,7 +125,7 @@ func (r *NovaCellReconciler) Reconcile(ctx context.Context, req ctrl.Request) (r panic(r) } // update the Ready condition based on the sub conditions - if allSubConditionIsTrue(instance.Status) { + if instance.Status.Conditions.AllSubConditionIsTrue() { instance.Status.Conditions.MarkTrue( condition.ReadyCondition, condition.ReadyMessage) } else { @@ -151,7 +151,7 @@ func (r *NovaCellReconciler) Reconcile(ctx context.Context, req ctrl.Request) (r } // For the compute config generation we need to read the input secrets - _, result, secret, err := ensureSecret( + _, result, secret, err := internalcommon.EnsureSecret( ctx, types.NamespacedName{Namespace: instance.Namespace, Name: instance.Spec.Secret}, requiredSecretFields, @@ -814,8 +814,8 @@ func (r *NovaCellReconciler) generateComputeConfigs( } configName := instance.GetName() + "-compute-config" - err := r.GenerateConfigs( - ctx, h, instance, configName, &hashes, templateParameters, extraData, cmLabels, map[string]string{}, + err := internalcommon.GenerateConfigs( + ctx, h, instance, configName, &hashes, templateParameters, extraData, cmLabels, novaAdditionalTemplates(), []string{}, "nova/compute", ) if err != nil { @@ -866,37 +866,14 @@ func (r *NovaCellReconciler) getVNCProxyURL( } func (r *NovaCellReconciler) findObjectsForSrc(ctx context.Context, src client.Object) []reconcile.Request { - requests := []reconcile.Request{} - - Log := r.GetLogger(ctx) - - for _, field := range cellWatchFields { - crList := &novav1.NovaCellList{} - listOps := &client.ListOptions{ - FieldSelector: fields.OneTermEqualSelector(field, src.GetName()), - Namespace: src.GetNamespace(), - } - err := r.Client.List(ctx, crList, listOps) - if err != nil { - Log.Error(err, fmt.Sprintf("listing %s for field: %s - %s", crList.GroupVersionKind().Kind, field, src.GetNamespace())) - return requests - } - - for _, item := range crList.Items { - Log.Info(fmt.Sprintf("input source %s changed, reconcile: %s - %s", src.GetName(), item.GetName(), item.GetNamespace())) - - requests = append(requests, - reconcile.Request{ - NamespacedName: types.NamespacedName{ - Name: item.GetName(), - Namespace: item.GetNamespace(), - }, - }, - ) - } - } - - return requests + return internalcommon.FindObjectsForSrcByField( + ctx, + r.GetLogger(ctx), + r.Client, + src, + cellWatchFields, + func() client.ObjectList { return &novav1.NovaCellList{} }, + ) } // fields to index to reconcile when change diff --git a/internal/controller/nova/novacompute_controller.go b/internal/controller/nova/novacompute_controller.go index 02163a10e..2b63368a9 100644 --- a/internal/controller/nova/novacompute_controller.go +++ b/internal/controller/nova/novacompute_controller.go @@ -23,7 +23,6 @@ import ( v1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/fields" "k8s.io/apimachinery/pkg/types" "k8s.io/utils/ptr" ctrl "sigs.k8s.io/controller-runtime" @@ -48,7 +47,7 @@ import ( util "github.com/openstack-k8s-operators/lib-common/modules/common/util" novav1 "github.com/openstack-k8s-operators/nova-operator/api/nova/v1beta1" - "github.com/openstack-k8s-operators/nova-operator/internal/nova" + internalcommon "github.com/openstack-k8s-operators/nova-operator/internal/common" novacompute "github.com/openstack-k8s-operators/nova-operator/internal/nova/compute" topologyv1 "github.com/openstack-k8s-operators/infra-operator/apis/topology/v1beta1" @@ -137,7 +136,7 @@ func (r *NovaComputeReconciler) Reconcile(ctx context.Context, req ctrl.Request) panic(r) } // update the Ready condition based on the sub conditions - if allSubConditionIsTrue(instance.Status) { + if instance.Status.Conditions.AllSubConditionIsTrue() { instance.Status.Conditions.MarkTrue( condition.ReadyCondition, condition.ReadyMessage) } else { @@ -184,7 +183,7 @@ func (r *NovaComputeReconciler) Reconcile(ctx context.Context, req ctrl.Request) NotificationTransportURLSelector, } - secretHash, result, secret, err := ensureSecret( + secretHash, result, secret, err := internalcommon.EnsureSecret( ctx, types.NamespacedName{Namespace: instance.Namespace, Name: instance.Spec.Secret}, requiredSecretFields, @@ -256,7 +255,7 @@ func (r *NovaComputeReconciler) Reconcile(ctx context.Context, req ctrl.Request) instance.Status.Conditions.MarkTrue(condition.ServiceConfigReadyCondition, condition.ServiceConfigReadyMessage) - serviceAnnotations, result, err := ensureNetworkAttachments(ctx, h, instance.Spec.NetworkAttachments, &instance.Status.Conditions, r.RequeueTimeout) + serviceAnnotations, result, err := internalcommon.EnsureNetworkAttachments(ctx, h, instance.Spec.NetworkAttachments, &instance.Status.Conditions, r.RequeueTimeout) if (err != nil || result != ctrl.Result{}) { return result, err } @@ -395,8 +394,8 @@ func (r *NovaComputeReconciler) generateConfigs( instance, labels.GetGroupLabel(NovaComputeLabelPrefix), map[string]string{}, ) - err := r.GenerateConfigs( - ctx, h, instance, nova.GetServiceConfigSecretName(instance.GetName()), hashes, templateParameters, extraData, cmLabels, map[string]string{}, + err := internalcommon.GenerateConfigs( + ctx, h, instance, internalcommon.GetServiceConfigSecretName(instance.GetName()), hashes, templateParameters, extraData, cmLabels, novaAdditionalTemplates(), []string{}, "nova/compute", ) return err @@ -415,7 +414,7 @@ func (r *NovaComputeReconciler) ensureDeployment( // // Handle Topology // - topology, err := ensureTopology( + topology, err := internalcommon.EnsureTopology( ctx, h, instance, // topologyHandler @@ -524,71 +523,25 @@ func getComputeServiceLabels(cell string) map[string]string { } func (r *NovaComputeReconciler) findObjectsForSrc(ctx context.Context, src client.Object) []reconcile.Request { - requests := []reconcile.Request{} - - Log := r.GetLogger(ctx) - - for _, field := range cmpWatchFields { - crList := &novav1.NovaComputeList{} - listOps := &client.ListOptions{ - FieldSelector: fields.OneTermEqualSelector(field, src.GetName()), - Namespace: src.GetNamespace(), - } - err := r.Client.List(ctx, crList, listOps) - if err != nil { - Log.Error(err, fmt.Sprintf("listing %s for field: %s - %s", crList.GroupVersionKind().Kind, field, src.GetNamespace())) - return requests - } - - for _, item := range crList.Items { - Log.Info(fmt.Sprintf("input source %s changed, reconcile: %s - %s", src.GetName(), item.GetName(), item.GetNamespace())) - - requests = append(requests, - reconcile.Request{ - NamespacedName: types.NamespacedName{ - Name: item.GetName(), - Namespace: item.GetNamespace(), - }, - }, - ) - } - } - - return requests + return internalcommon.FindObjectsForSrcByField( + ctx, + r.GetLogger(ctx), + r.Client, + src, + cmpWatchFields, + func() client.ObjectList { return &novav1.NovaComputeList{} }, + ) } func (r *NovaComputeReconciler) findObjectsWithAppSelectorLabelInNamespace(ctx context.Context, src client.Object) []reconcile.Request { - requests := []reconcile.Request{} - - Log := r.GetLogger(ctx) - - // if the endpoint has the service label and its in our endpointList, reconcile the CR in the namespace - if svc, ok := src.GetLabels()[common.AppSelector]; ok && util.StringInSlice(svc, endpointList) { - crList := &novav1.NovaComputeList{} - listOps := &client.ListOptions{ - Namespace: src.GetNamespace(), - } - err := r.Client.List(ctx, crList, listOps) - if err != nil { - Log.Error(err, fmt.Sprintf("listing %s for namespace: %s", crList.GroupVersionKind().Kind, src.GetNamespace())) - return requests - } - - for _, item := range crList.Items { - Log.Info(fmt.Sprintf("input source %s changed, reconcile: %s - %s", src.GetName(), item.GetName(), item.GetNamespace())) - - requests = append(requests, - reconcile.Request{ - NamespacedName: types.NamespacedName{ - Name: item.GetName(), - Namespace: item.GetNamespace(), - }, - }, - ) - } - } - - return requests + return internalcommon.FindObjectsWithAppSelectorLabelInNamespace( + ctx, + r.GetLogger(ctx), + r.Client, + src, + endpointList, + func() client.ObjectList { return &novav1.NovaComputeList{} }, + ) } // fields to index to reconcile when change diff --git a/internal/controller/nova/novaconductor_controller.go b/internal/controller/nova/novaconductor_controller.go index fbc6d40c0..b71f141f2 100644 --- a/internal/controller/nova/novaconductor_controller.go +++ b/internal/controller/nova/novaconductor_controller.go @@ -24,7 +24,6 @@ import ( batchv1 "k8s.io/api/batch/v1" corev1 "k8s.io/api/core/v1" k8s_errors "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/fields" "k8s.io/apimachinery/pkg/types" "k8s.io/utils/ptr" ctrl "sigs.k8s.io/controller-runtime" @@ -54,6 +53,7 @@ import ( util "github.com/openstack-k8s-operators/lib-common/modules/common/util" mariadbv1 "github.com/openstack-k8s-operators/mariadb-operator/api/v1beta1" novav1 "github.com/openstack-k8s-operators/nova-operator/api/nova/v1beta1" + internalcommon "github.com/openstack-k8s-operators/nova-operator/internal/common" novaconductor "github.com/openstack-k8s-operators/nova-operator/internal/nova/conductor" ) @@ -139,7 +139,7 @@ func (r *NovaConductorReconciler) Reconcile(ctx context.Context, req ctrl.Reques panic(r) } // update the Ready condition based on the sub conditions - if allSubConditionIsTrue(instance.Status) { + if instance.Status.Conditions.AllSubConditionIsTrue() { instance.Status.Conditions.MarkTrue( condition.ReadyCondition, condition.ReadyMessage) } else { @@ -204,7 +204,7 @@ func (r *NovaConductorReconciler) Reconcile(ctx context.Context, req ctrl.Reques NotificationTransportURLSelector, } - secretHash, result, secret, err := ensureSecret( + secretHash, result, secret, err := internalcommon.EnsureSecret( ctx, types.NamespacedName{Namespace: instance.Namespace, Name: instance.Spec.Secret}, requiredSecretFields, @@ -296,7 +296,7 @@ func (r *NovaConductorReconciler) Reconcile(ctx context.Context, req ctrl.Reques instance.Status.Conditions.MarkTrue(condition.ServiceConfigReadyCondition, condition.ServiceConfigReadyMessage) - serviceAnnotations, result, err := ensureNetworkAttachments(ctx, h, instance.Spec.NetworkAttachments, &instance.Status.Conditions, r.RequeueTimeout) + serviceAnnotations, result, err := internalcommon.EnsureNetworkAttachments(ctx, h, instance.Spec.NetworkAttachments, &instance.Status.Conditions, r.RequeueTimeout) if (err != nil || result != ctrl.Result{}) { return result, err } @@ -515,8 +515,8 @@ func (r *NovaConductorReconciler) generateConfigs( cmLabels := labels.GetLabels( instance, labels.GetGroupLabel(NovaConductorLabelPrefix), map[string]string{}) - return r.GenerateConfigsWithScripts( - ctx, h, instance, hashes, templateParameters, extraData, cmLabels, map[string]string{}, + return internalcommon.GenerateConfigsWithScripts( + ctx, h, instance, hashes, templateParameters, extraData, cmLabels, novaAdditionalTemplates(), []string{}, "nova/conductor", ) } @@ -580,7 +580,7 @@ func (r *NovaConductorReconciler) ensureDeployment( // // Handle Topology // - topology, err := ensureTopology( + topology, err := internalcommon.EnsureTopology( ctx, h, instance, // topologyHandler @@ -717,71 +717,25 @@ func (r *NovaConductorReconciler) cleanServiceFromNovaDb( } func (r *NovaConductorReconciler) findObjectsForSrc(ctx context.Context, src client.Object) []reconcile.Request { - requests := []reconcile.Request{} - - Log := r.GetLogger(ctx) - - for _, field := range cdWatchFields { - crList := &novav1.NovaConductorList{} - listOps := &client.ListOptions{ - FieldSelector: fields.OneTermEqualSelector(field, src.GetName()), - Namespace: src.GetNamespace(), - } - err := r.Client.List(ctx, crList, listOps) - if err != nil { - Log.Error(err, fmt.Sprintf("listing %s for field: %s - %s", crList.GroupVersionKind().Kind, field, src.GetNamespace())) - return requests - } - - for _, item := range crList.Items { - Log.Info(fmt.Sprintf("input source %s changed, reconcile: %s - %s", src.GetName(), item.GetName(), item.GetNamespace())) - - requests = append(requests, - reconcile.Request{ - NamespacedName: types.NamespacedName{ - Name: item.GetName(), - Namespace: item.GetNamespace(), - }, - }, - ) - } - } - - return requests + return internalcommon.FindObjectsForSrcByField( + ctx, + r.GetLogger(ctx), + r.Client, + src, + cdWatchFields, + func() client.ObjectList { return &novav1.NovaConductorList{} }, + ) } func (r *NovaConductorReconciler) findObjectsWithAppSelectorLabelInNamespace(ctx context.Context, src client.Object) []reconcile.Request { - requests := []reconcile.Request{} - - Log := r.GetLogger(ctx) - - // if the endpoint has the service label and its in our endpointList, reconcile the CR in the namespace - if svc, ok := src.GetLabels()[common.AppSelector]; ok && util.StringInSlice(svc, endpointList) { - crList := &novav1.NovaConductorList{} - listOps := &client.ListOptions{ - Namespace: src.GetNamespace(), - } - err := r.Client.List(ctx, crList, listOps) - if err != nil { - Log.Error(err, fmt.Sprintf("listing %s for namespace: %s", crList.GroupVersionKind().Kind, src.GetNamespace())) - return requests - } - - for _, item := range crList.Items { - Log.Info(fmt.Sprintf("input source %s changed, reconcile: %s - %s", src.GetName(), item.GetName(), item.GetNamespace())) - - requests = append(requests, - reconcile.Request{ - NamespacedName: types.NamespacedName{ - Name: item.GetName(), - Namespace: item.GetNamespace(), - }, - }, - ) - } - } - - return requests + return internalcommon.FindObjectsWithAppSelectorLabelInNamespace( + ctx, + r.GetLogger(ctx), + r.Client, + src, + endpointList, + func() client.ObjectList { return &novav1.NovaConductorList{} }, + ) } // fields to index to reconcile when change diff --git a/internal/controller/nova/novametadata_controller.go b/internal/controller/nova/novametadata_controller.go index ebcb87d11..f06c0f455 100644 --- a/internal/controller/nova/novametadata_controller.go +++ b/internal/controller/nova/novametadata_controller.go @@ -24,7 +24,6 @@ import ( v1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/fields" "k8s.io/apimachinery/pkg/types" "k8s.io/utils/ptr" ctrl "sigs.k8s.io/controller-runtime" @@ -54,7 +53,7 @@ import ( util "github.com/openstack-k8s-operators/lib-common/modules/common/util" mariadbv1 "github.com/openstack-k8s-operators/mariadb-operator/api/v1beta1" novav1 "github.com/openstack-k8s-operators/nova-operator/api/nova/v1beta1" - "github.com/openstack-k8s-operators/nova-operator/internal/nova" + internalcommon "github.com/openstack-k8s-operators/nova-operator/internal/common" novametadata "github.com/openstack-k8s-operators/nova-operator/internal/nova/metadata" k8s_errors "k8s.io/apimachinery/pkg/api/errors" ) @@ -140,7 +139,7 @@ func (r *NovaMetadataReconciler) Reconcile(ctx context.Context, req ctrl.Request panic(r) } // update the Ready condition based on the sub conditions - if allSubConditionIsTrue(instance.Status) { + if instance.Status.Conditions.AllSubConditionIsTrue() { instance.Status.Conditions.MarkTrue( condition.ReadyCondition, condition.ReadyMessage) } else { @@ -201,7 +200,7 @@ func (r *NovaMetadataReconciler) Reconcile(ctx context.Context, req ctrl.Request TransportURLSelector, } - secretHash, result, secret, err := ensureSecret( + secretHash, result, secret, err := internalcommon.EnsureSecret( ctx, types.NamespacedName{Namespace: instance.Namespace, Name: instance.Spec.Secret}, expectedSelectors, @@ -324,7 +323,7 @@ func (r *NovaMetadataReconciler) Reconcile(ctx context.Context, req ctrl.Request instance.Status.Conditions.MarkTrue(condition.ServiceConfigReadyCondition, condition.ServiceConfigReadyMessage) - serviceAnnotations, result, err := ensureNetworkAttachments(ctx, h, instance.Spec.NetworkAttachments, &instance.Status.Conditions, r.RequeueTimeout) + serviceAnnotations, result, err := internalcommon.EnsureNetworkAttachments(ctx, h, instance.Spec.NetworkAttachments, &instance.Status.Conditions, r.RequeueTimeout) if (err != nil || result != ctrl.Result{}) { return result, err } @@ -576,9 +575,9 @@ func (r *NovaMetadataReconciler) generateConfigs( instance, labels.GetGroupLabel(NovaMetadataLabelPrefix), map[string]string{}, ) - err = r.GenerateConfigs( - ctx, h, instance, nova.GetServiceConfigSecretName(instance.GetName()), - hashes, templateParameters, extraData, cmLabels, map[string]string{}, + err = internalcommon.GenerateConfigs( + ctx, h, instance, internalcommon.GetServiceConfigSecretName(instance.GetName()), + hashes, templateParameters, extraData, cmLabels, novaAdditionalTemplates(), []string{"ssl.conf"}, "nova/metadata", ) return err @@ -598,7 +597,7 @@ func (r *NovaMetadataReconciler) ensureDeployment( // // Handle Topology // - topology, err := ensureTopology( + topology, err := internalcommon.EnsureTopology( ctx, h, instance, // topologyHandler @@ -917,71 +916,25 @@ func (r *NovaMetadataReconciler) generateNeutronConfigs( } func (r *NovaMetadataReconciler) findObjectsForSrc(ctx context.Context, src client.Object) []reconcile.Request { - requests := []reconcile.Request{} - - Log := r.GetLogger(ctx) - - for _, field := range metaWatchFields { - crList := &novav1.NovaMetadataList{} - listOps := &client.ListOptions{ - FieldSelector: fields.OneTermEqualSelector(field, src.GetName()), - Namespace: src.GetNamespace(), - } - err := r.Client.List(ctx, crList, listOps) - if err != nil { - Log.Error(err, fmt.Sprintf("listing %s for field: %s - %s", crList.GroupVersionKind().Kind, field, src.GetNamespace())) - return requests - } - - for _, item := range crList.Items { - Log.Info(fmt.Sprintf("input source %s changed, reconcile: %s - %s", src.GetName(), item.GetName(), item.GetNamespace())) - - requests = append(requests, - reconcile.Request{ - NamespacedName: types.NamespacedName{ - Name: item.GetName(), - Namespace: item.GetNamespace(), - }, - }, - ) - } - } - - return requests + return internalcommon.FindObjectsForSrcByField( + ctx, + r.GetLogger(ctx), + r.Client, + src, + metaWatchFields, + func() client.ObjectList { return &novav1.NovaMetadataList{} }, + ) } func (r *NovaMetadataReconciler) findObjectsWithAppSelectorLabelInNamespace(ctx context.Context, src client.Object) []reconcile.Request { - requests := []reconcile.Request{} - - Log := r.GetLogger(ctx) - - // if the endpoint has the service label and its in our endpointList, reconcile the CR in the namespace - if svc, ok := src.GetLabels()[common.AppSelector]; ok && util.StringInSlice(svc, endpointList) { - crList := &novav1.NovaMetadataList{} - listOps := &client.ListOptions{ - Namespace: src.GetNamespace(), - } - err := r.Client.List(ctx, crList, listOps) - if err != nil { - Log.Error(err, fmt.Sprintf("listing %s for namespace: %s", crList.GroupVersionKind().Kind, src.GetNamespace())) - return requests - } - - for _, item := range crList.Items { - Log.Info(fmt.Sprintf("input source %s changed, reconcile: %s - %s", src.GetName(), item.GetName(), item.GetNamespace())) - - requests = append(requests, - reconcile.Request{ - NamespacedName: types.NamespacedName{ - Name: item.GetName(), - Namespace: item.GetNamespace(), - }, - }, - ) - } - } - - return requests + return internalcommon.FindObjectsWithAppSelectorLabelInNamespace( + ctx, + r.GetLogger(ctx), + r.Client, + src, + endpointList, + func() client.ObjectList { return &novav1.NovaMetadataList{} }, + ) } // fields to index to reconcile when change diff --git a/internal/controller/nova/novanovncproxy_controller.go b/internal/controller/nova/novanovncproxy_controller.go index 0f35d7e50..f742a92b3 100644 --- a/internal/controller/nova/novanovncproxy_controller.go +++ b/internal/controller/nova/novanovncproxy_controller.go @@ -22,7 +22,6 @@ import ( v1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/fields" "k8s.io/apimachinery/pkg/types" "k8s.io/utils/ptr" ctrl "sigs.k8s.io/controller-runtime" @@ -51,7 +50,7 @@ import ( util "github.com/openstack-k8s-operators/lib-common/modules/common/util" mariadbv1 "github.com/openstack-k8s-operators/mariadb-operator/api/v1beta1" novav1 "github.com/openstack-k8s-operators/nova-operator/api/nova/v1beta1" - "github.com/openstack-k8s-operators/nova-operator/internal/nova" + internalcommon "github.com/openstack-k8s-operators/nova-operator/internal/common" "github.com/openstack-k8s-operators/nova-operator/internal/nova/novncproxy" k8s_errors "k8s.io/apimachinery/pkg/api/errors" ) @@ -140,7 +139,7 @@ func (r *NovaNoVNCProxyReconciler) Reconcile(ctx context.Context, req ctrl.Reque panic(r) } // update the Ready condition based on the sub conditions - if allSubConditionIsTrue(instance.Status) { + if instance.Status.Conditions.AllSubConditionIsTrue() { instance.Status.Conditions.MarkTrue( condition.ReadyCondition, condition.ReadyMessage) } else { @@ -195,7 +194,7 @@ func (r *NovaNoVNCProxyReconciler) Reconcile(ctx context.Context, req ctrl.Reque } hashes["endpointUrlsHash"] = env.SetValue(endpointUrlsHash) - secretHash, result, secret, err := ensureSecret( + secretHash, result, secret, err := internalcommon.EnsureSecret( ctx, types.NamespacedName{Namespace: instance.Namespace, Name: instance.Spec.Secret}, []string{ @@ -346,7 +345,7 @@ func (r *NovaNoVNCProxyReconciler) Reconcile(ctx context.Context, req ctrl.Reque instance.Status.Conditions.MarkTrue(condition.ServiceConfigReadyCondition, condition.ServiceConfigReadyMessage) - serviceAnnotations, result, err := ensureNetworkAttachments(ctx, h, instance.Spec.NetworkAttachments, &instance.Status.Conditions, r.RequeueTimeout) + serviceAnnotations, result, err := internalcommon.EnsureNetworkAttachments(ctx, h, instance.Spec.NetworkAttachments, &instance.Status.Conditions, r.RequeueTimeout) if (err != nil || result != ctrl.Result{}) { return result, err } @@ -533,9 +532,9 @@ func (r *NovaNoVNCProxyReconciler) generateConfigs( instance, labels.GetGroupLabel(NovaNoVNCProxyLabelPrefix), map[string]string{}, ) - err = r.GenerateConfigs( - ctx, h, instance, nova.GetServiceConfigSecretName(instance.GetName()), - hashes, templateParameters, extraData, cmLabels, map[string]string{}, + err = internalcommon.GenerateConfigs( + ctx, h, instance, internalcommon.GetServiceConfigSecretName(instance.GetName()), + hashes, templateParameters, extraData, cmLabels, novaAdditionalTemplates(), []string{}, "nova/novncproxy", ) return err @@ -555,7 +554,7 @@ func (r *NovaNoVNCProxyReconciler) ensureDeployment( // // Handle Topology // - topology, err := ensureTopology( + topology, err := internalcommon.EnsureTopology( ctx, h, instance, // topologyHandler @@ -788,71 +787,25 @@ func getNoVNCProxyServiceLabels(cell string) map[string]string { } func (r *NovaNoVNCProxyReconciler) findObjectsForSrc(ctx context.Context, src client.Object) []reconcile.Request { - requests := []reconcile.Request{} - - Log := r.GetLogger(ctx) - - for _, field := range noVNCProxyWatchFields { - crList := &novav1.NovaNoVNCProxyList{} - listOps := &client.ListOptions{ - FieldSelector: fields.OneTermEqualSelector(field, src.GetName()), - Namespace: src.GetNamespace(), - } - err := r.Client.List(ctx, crList, listOps) - if err != nil { - Log.Error(err, fmt.Sprintf("listing %s for field: %s - %s", crList.GroupVersionKind().Kind, field, src.GetNamespace())) - return requests - } - - for _, item := range crList.Items { - Log.Info(fmt.Sprintf("input source %s changed, reconcile: %s - %s", src.GetName(), item.GetName(), item.GetNamespace())) - - requests = append(requests, - reconcile.Request{ - NamespacedName: types.NamespacedName{ - Name: item.GetName(), - Namespace: item.GetNamespace(), - }, - }, - ) - } - } - - return requests + return internalcommon.FindObjectsForSrcByField( + ctx, + r.GetLogger(ctx), + r.Client, + src, + noVNCProxyWatchFields, + func() client.ObjectList { return &novav1.NovaNoVNCProxyList{} }, + ) } func (r *NovaNoVNCProxyReconciler) findObjectsWithAppSelectorLabelInNamespace(ctx context.Context, src client.Object) []reconcile.Request { - requests := []reconcile.Request{} - - Log := r.GetLogger(ctx) - - // if the endpoint has the service label and its in our endpointList, reconcile the CR in the namespace - if svc, ok := src.GetLabels()[common.AppSelector]; ok && util.StringInSlice(svc, endpointList) { - crList := &novav1.NovaNoVNCProxyList{} - listOps := &client.ListOptions{ - Namespace: src.GetNamespace(), - } - err := r.Client.List(ctx, crList, listOps) - if err != nil { - Log.Error(err, fmt.Sprintf("listing %s for namespace: %s", crList.GroupVersionKind().Kind, src.GetNamespace())) - return requests - } - - for _, item := range crList.Items { - Log.Info(fmt.Sprintf("input source %s changed, reconcile: %s - %s", src.GetName(), item.GetName(), item.GetNamespace())) - - requests = append(requests, - reconcile.Request{ - NamespacedName: types.NamespacedName{ - Name: item.GetName(), - Namespace: item.GetNamespace(), - }, - }, - ) - } - } - - return requests + return internalcommon.FindObjectsWithAppSelectorLabelInNamespace( + ctx, + r.GetLogger(ctx), + r.Client, + src, + endpointList, + func() client.ObjectList { return &novav1.NovaNoVNCProxyList{} }, + ) } // fields to index to reconcile when change diff --git a/internal/controller/nova/novascheduler_controller.go b/internal/controller/nova/novascheduler_controller.go index 1c987e777..c7aa2eaa3 100644 --- a/internal/controller/nova/novascheduler_controller.go +++ b/internal/controller/nova/novascheduler_controller.go @@ -23,7 +23,6 @@ import ( v1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" k8s_errors "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/fields" "k8s.io/apimachinery/pkg/types" "k8s.io/utils/ptr" ctrl "sigs.k8s.io/controller-runtime" @@ -52,7 +51,7 @@ import ( topologyv1 "github.com/openstack-k8s-operators/infra-operator/apis/topology/v1beta1" novav1 "github.com/openstack-k8s-operators/nova-operator/api/nova/v1beta1" - "github.com/openstack-k8s-operators/nova-operator/internal/nova" + internalcommon "github.com/openstack-k8s-operators/nova-operator/internal/common" novascheduler "github.com/openstack-k8s-operators/nova-operator/internal/nova/scheduler" ) @@ -138,7 +137,7 @@ func (r *NovaSchedulerReconciler) Reconcile(ctx context.Context, req ctrl.Reques panic(r) } // update the Ready condition based on the sub conditions - if allSubConditionIsTrue(instance.Status) { + if instance.Status.Conditions.AllSubConditionIsTrue() { instance.Status.Conditions.MarkTrue( condition.ReadyCondition, condition.ReadyMessage) } else { @@ -205,7 +204,7 @@ func (r *NovaSchedulerReconciler) Reconcile(ctx context.Context, req ctrl.Reques NotificationTransportURLSelector, } - secretHash, result, secret, err := ensureSecret( + secretHash, result, secret, err := internalcommon.EnsureSecret( ctx, types.NamespacedName{Namespace: instance.Namespace, Name: instance.Spec.Secret}, requiredSecretFields, @@ -306,7 +305,7 @@ func (r *NovaSchedulerReconciler) Reconcile(ctx context.Context, req ctrl.Reques instance.Status.Conditions.MarkTrue(condition.ServiceConfigReadyCondition, condition.ServiceConfigReadyMessage) - serviceAnnotations, result, err := ensureNetworkAttachments(ctx, h, instance.Spec.NetworkAttachments, &instance.Status.Conditions, r.RequeueTimeout) + serviceAnnotations, result, err := internalcommon.EnsureNetworkAttachments(ctx, h, instance.Spec.NetworkAttachments, &instance.Status.Conditions, r.RequeueTimeout) if (err != nil || result != ctrl.Result{}) { return result, err } @@ -333,71 +332,25 @@ func (r *NovaSchedulerReconciler) Reconcile(ctx context.Context, req ctrl.Reques } func (r *NovaSchedulerReconciler) findObjectsForSrc(ctx context.Context, src client.Object) []reconcile.Request { - requests := []reconcile.Request{} - - Log := r.GetLogger(ctx) - - for _, field := range schedulerWatchFields { - crList := &novav1.NovaSchedulerList{} - listOps := &client.ListOptions{ - FieldSelector: fields.OneTermEqualSelector(field, src.GetName()), - Namespace: src.GetNamespace(), - } - err := r.Client.List(ctx, crList, listOps) - if err != nil { - Log.Error(err, fmt.Sprintf("listing %s for field: %s - %s", crList.GroupVersionKind().Kind, field, src.GetNamespace())) - return requests - } - - for _, item := range crList.Items { - Log.Info(fmt.Sprintf("input source %s changed, reconcile: %s - %s", src.GetName(), item.GetName(), item.GetNamespace())) - - requests = append(requests, - reconcile.Request{ - NamespacedName: types.NamespacedName{ - Name: item.GetName(), - Namespace: item.GetNamespace(), - }, - }, - ) - } - } - - return requests + return internalcommon.FindObjectsForSrcByField( + ctx, + r.GetLogger(ctx), + r.Client, + src, + schedulerWatchFields, + func() client.ObjectList { return &novav1.NovaSchedulerList{} }, + ) } func (r *NovaSchedulerReconciler) findObjectsWithAppSelectorLabelInNamespace(ctx context.Context, src client.Object) []reconcile.Request { - requests := []reconcile.Request{} - - Log := r.GetLogger(ctx) - - // if the endpoint has the service label and its in our endpointList, reconcile the CR in the namespace - if svc, ok := src.GetLabels()[common.AppSelector]; ok && util.StringInSlice(svc, endpointList) { - crList := &novav1.NovaSchedulerList{} - listOps := &client.ListOptions{ - Namespace: src.GetNamespace(), - } - err := r.Client.List(ctx, crList, listOps) - if err != nil { - Log.Error(err, fmt.Sprintf("listing %s for namespace: %s", crList.GroupVersionKind().Kind, src.GetNamespace())) - return requests - } - - for _, item := range crList.Items { - Log.Info(fmt.Sprintf("input source %s changed, reconcile: %s - %s", src.GetName(), item.GetName(), item.GetNamespace())) - - requests = append(requests, - reconcile.Request{ - NamespacedName: types.NamespacedName{ - Name: item.GetName(), - Namespace: item.GetNamespace(), - }, - }, - ) - } - } - - return requests + return internalcommon.FindObjectsWithAppSelectorLabelInNamespace( + ctx, + r.GetLogger(ctx), + r.Client, + src, + endpointList, + func() client.ObjectList { return &novav1.NovaSchedulerList{} }, + ) } // fields to index to reconcile when change @@ -635,9 +588,9 @@ func (r *NovaSchedulerReconciler) generateConfigs( instance, labels.GetGroupLabel(NovaSchedulerLabelPrefix), map[string]string{}, ) - return r.GenerateConfigs( - ctx, h, instance, nova.GetServiceConfigSecretName(instance.GetName()), - hashes, templateParameters, extraData, cmLabels, map[string]string{}, + return internalcommon.GenerateConfigs( + ctx, h, instance, internalcommon.GetServiceConfigSecretName(instance.GetName()), + hashes, templateParameters, extraData, cmLabels, novaAdditionalTemplates(), []string{}, "nova/scheduler", ) } @@ -685,7 +638,7 @@ func (r *NovaSchedulerReconciler) ensureDeployment( // // Handle Topology // - topology, err := ensureTopology( + topology, err := internalcommon.EnsureTopology( ctx, h, instance, // topologyHandler diff --git a/internal/controller/placement/api_controller.go b/internal/controller/placement/api_controller.go index 67c15533f..bda758c9a 100644 --- a/internal/controller/placement/api_controller.go +++ b/internal/controller/placement/api_controller.go @@ -19,16 +19,11 @@ package controller import ( "context" - "errors" "fmt" "maps" "slices" - "time" - "k8s.io/apimachinery/pkg/fields" - "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" - "k8s.io/client-go/kubernetes" "k8s.io/utils/ptr" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/builder" @@ -40,7 +35,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/reconcile" "github.com/go-logr/logr" - networkv1 "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1" topologyv1 "github.com/openstack-k8s-operators/infra-operator/apis/topology/v1beta1" keystonev1 "github.com/openstack-k8s-operators/keystone-operator/api/v1beta1" common "github.com/openstack-k8s-operators/lib-common/modules/common" @@ -61,6 +55,7 @@ import ( mariadbv1 "github.com/openstack-k8s-operators/mariadb-operator/api/v1beta1" placementv1 "github.com/openstack-k8s-operators/nova-operator/api/placement/v1beta1" + internalcommon "github.com/openstack-k8s-operators/nova-operator/internal/common" placement "github.com/openstack-k8s-operators/nova-operator/internal/placement" appsv1 "k8s.io/api/apps/v1" @@ -70,91 +65,6 @@ import ( k8s_errors "k8s.io/apimachinery/pkg/api/errors" ) -// Static errors for Application Credential handling -var ( - ErrACSecretNotFound = errors.New("ApplicationCredential secret not found") - ErrACSecretMissingKeys = errors.New("ApplicationCredential secret missing required keys") -) - -type conditionUpdater interface { - Set(c *condition.Condition) - MarkTrue(t condition.Type, messageFormat string, messageArgs ...any) -} - -// GetSecret interface defines methods for objects that can provide secret names -type GetSecret interface { - GetSecret() string - client.Object -} - -// ensureSecret - ensures that the Secret object exists and the expected fields -// are in the Secret. It returns a hash of the values of the expected fields. -func ensureSecret( - ctx context.Context, - secretName types.NamespacedName, - expectedFields []string, - reader client.Reader, - conditionUpdater conditionUpdater, -) (string, ctrl.Result, corev1.Secret, error) { - secret := &corev1.Secret{} - err := reader.Get(ctx, secretName, secret) - if err != nil { - if k8s_errors.IsNotFound(err) { - // This is currently only used to acquire the password secret, which should have been - // manually created by the user and referenced in the spec, so we treat this as a - // warning because it means that the service will not be able to start. - conditionUpdater.Set(condition.FalseCondition( - condition.InputReadyCondition, - condition.ErrorReason, - condition.SeverityWarning, - "%s", fmt.Sprintf("Input data resources missing: %s", "secret/"+secretName.Name))) - return "", - ctrl.Result{}, - *secret, - fmt.Errorf("%w: Secret %s not found", err, secretName) - } - conditionUpdater.Set(condition.FalseCondition( - condition.InputReadyCondition, - condition.ErrorReason, - condition.SeverityWarning, - condition.InputReadyErrorMessage, - err.Error())) - return "", ctrl.Result{}, *secret, err - } - - // collect the secret values the caller expects to exist - values := [][]byte{} - for _, field := range expectedFields { - val, ok := secret.Data[field] - if !ok { - err := fmt.Errorf("%w: field '%s' not found in secret/%s", util.ErrFieldNotFound, field, secretName.Name) - conditionUpdater.Set(condition.FalseCondition( - condition.InputReadyCondition, - condition.ErrorReason, - condition.SeverityWarning, - condition.InputReadyErrorMessage, - err.Error())) - return "", ctrl.Result{}, *secret, err - } - values = append(values, val) - } - - // TODO(gibi): Do we need to watch the Secret for changes? - - hash, err := util.ObjectHash(values) - if err != nil { - conditionUpdater.Set(condition.FalseCondition( - condition.InputReadyCondition, - condition.ErrorReason, - condition.SeverityWarning, - condition.InputReadyErrorMessage, - err.Error())) - return "", ctrl.Result{}, *secret, err - } - - return hash, ctrl.Result{}, *secret, nil -} - // GetLogger returns a logger object with a prefix of "controller.name" and additional controller context fields func (r *PlacementAPIReconciler) GetLogger(ctx context.Context) logr.Logger { return log.FromContext(ctx).WithName("Controllers").WithName("PlacementAPI") @@ -162,9 +72,7 @@ func (r *PlacementAPIReconciler) GetLogger(ctx context.Context) logr.Logger { // PlacementAPIReconciler reconciles a PlacementAPI object type PlacementAPIReconciler struct { - client.Client - Kclient kubernetes.Interface - Scheme *runtime.Scheme + internalcommon.ReconcilerBase } // +kubebuilder:rbac:groups=placement.openstack.org,resources=placementapis,verbs=get;list;watch;create;update;patch;delete @@ -200,7 +108,7 @@ func (r *PlacementAPIReconciler) Reconcile(ctx context.Context, req ctrl.Request // Fetch the PlacementAPI instance instance := &placementv1.PlacementAPI{} - err := r.Get(ctx, req.NamespacedName, instance) + err := r.Client.Get(ctx, req.NamespacedName, instance) if err != nil { if k8s_errors.IsNotFound(err) { // Request object not found, could have been deleted after reconcile request. @@ -307,34 +215,31 @@ func (r *PlacementAPIReconciler) Reconcile(ctx context.Context, req ctrl.Request // // check for required OpenStack secret holding passwords for service/admin user and add hash to the vars map // - hash, result, secret, err := ensureSecret( + // EnsureSecret is shared with nova via internal/common. Compared to the former + // placement-local ensureSecret, a missing Secret returns + // (Result{RequeueAfter: r.RequeueTimeout}, nil) instead of a NotFound error with + // an empty Result, and sets InputReady before requeue. The caller below only logs + // and returns the Result without treating a missing Secret as a reconcile error. + hash, result, secret, err := internalcommon.EnsureSecret( ctx, types.NamespacedName{Namespace: instance.Namespace, Name: instance.Spec.Secret}, []string{ instance.Spec.PasswordSelectors.Service, }, h.GetClient(), - &instance.Status.Conditions) + &instance.Status.Conditions, + r.RequeueTimeout, + ) if err != nil { - if k8s_errors.IsNotFound(err) { - // Since the service secret should have been manually created by the user and referenced in the spec, - // we treat this as a warning because it means that the service will not be able to start. - Log.Info(fmt.Sprintf("OpenStack secret %s not found", instance.Spec.Secret)) - instance.Status.Conditions.Set(condition.FalseCondition( - condition.InputReadyCondition, - condition.ErrorReason, - condition.SeverityWarning, - condition.InputReadyWaitingMessage)) - return ctrl.Result{RequeueAfter: time.Second * 10}, nil - } - instance.Status.Conditions.Set(condition.FalseCondition( - condition.InputReadyCondition, - condition.ErrorReason, - condition.SeverityWarning, - condition.InputReadyErrorMessage, - err.Error())) return result, err } + if (result != ctrl.Result{}) { + // Since the service secret should have been manually created by the user and referenced in the spec, + // we treat this as a warning because it means that the service will not be able to start. + // InputReady condition and requeue are already set by EnsureSecret above. + Log.Info(fmt.Sprintf("OpenStack secret %s not found", instance.Spec.Secret)) + return result, nil + } configMapVars[instance.Spec.Secret] = env.SetValue(hash) // all our input checks out so report InputReady @@ -374,6 +279,15 @@ func (r *PlacementAPIReconciler) Reconcile(ctx context.Context, req ctrl.Request err = r.generateServiceConfigMaps(ctx, h, instance, secret, &configMapVars, db) if err != nil { + if k8s_errors.IsNotFound(err) && instance.Spec.Auth.ApplicationCredentialSecret != "" { + Log.Info("ApplicationCredential secret not found, waiting", "secret", instance.Spec.Auth.ApplicationCredentialSecret) + instance.Status.Conditions.Set(condition.FalseCondition( + condition.InputReadyCondition, + condition.RequestedReason, + condition.SeverityInfo, + condition.InputReadyWaitingMessage)) + return ctrl.Result{RequeueAfter: r.RequeueTimeout}, nil + } instance.Status.Conditions.Set(condition.FalseCondition( condition.ServiceConfigReadyCondition, condition.ErrorReason, @@ -472,7 +386,7 @@ func (r *PlacementAPIReconciler) Reconcile(ctx context.Context, req ctrl.Request instance.Status.Conditions.MarkTrue(condition.ServiceConfigReadyCondition, condition.ServiceConfigReadyMessage) - serviceAnnotations, result, err := r.ensureNetworkAttachments(ctx, h, instance) + serviceAnnotations, result, err := internalcommon.EnsureNetworkAttachments(ctx, h, instance.Spec.NetworkAttachments, &instance.Status.Conditions, r.RequeueTimeout) if (err != nil || result != ctrl.Result{}) { return result, err } @@ -660,54 +574,6 @@ func (r *PlacementAPIReconciler) ensureServiceExposed( return apiEndpoints, ctrl.Result{}, nil } -func (r *PlacementAPIReconciler) ensureNetworkAttachments( - ctx context.Context, - h *helper.Helper, - instance *placementv1.PlacementAPI, -) (map[string]string, ctrl.Result, error) { - var nadAnnotations map[string]string - var err error - - Log := r.GetLogger(ctx) - // networks to attach to - nadList := []networkv1.NetworkAttachmentDefinition{} - for _, netAtt := range instance.Spec.NetworkAttachments { - nad, err := nad.GetNADWithName(ctx, h, netAtt, instance.Namespace) - if err != nil { - if k8s_errors.IsNotFound(err) { - // Since the net-attach-def CR should have been manually created by the user and referenced in the spec, - // we treat this as a warning because it means that the service will not be able to start. - Log.Info(fmt.Sprintf("network-attachment-definition %s not found", netAtt)) - instance.Status.Conditions.Set(condition.FalseCondition( - condition.NetworkAttachmentsReadyCondition, - condition.ErrorReason, - condition.SeverityWarning, - condition.NetworkAttachmentsReadyWaitingMessage, - netAtt)) - return nadAnnotations, ctrl.Result{RequeueAfter: time.Second * 10}, nil - } - instance.Status.Conditions.Set(condition.FalseCondition( - condition.NetworkAttachmentsReadyCondition, - condition.ErrorReason, - condition.SeverityWarning, - condition.NetworkAttachmentsReadyErrorMessage, - err.Error())) - return nadAnnotations, ctrl.Result{}, err - } - - if nad != nil { - nadList = append(nadList, *nad) - } - } - - nadAnnotations, err = nad.EnsureNetworksAnnotation(nadList) - if err != nil { - return nadAnnotations, ctrl.Result{}, fmt.Errorf("failed create network annotation from %s: %w", - instance.Spec.NetworkAttachments, err) - } - return nadAnnotations, ctrl.Result{}, nil -} - func (r *PlacementAPIReconciler) ensureKeystoneServiceUser( ctx context.Context, h *helper.Helper, @@ -726,7 +592,7 @@ func (r *PlacementAPIReconciler) ensureKeystoneServiceUser( PasswordSelector: instance.Spec.PasswordSelectors.Service, } serviceLabels := getServiceLabels(instance) - ksSvc := keystonev1.NewKeystoneService(ksSvcSpec, instance.Namespace, serviceLabels, time.Duration(10)*time.Second) + ksSvc := keystonev1.NewKeystoneService(ksSvcSpec, instance.Namespace, serviceLabels, r.RequeueTimeout) _, err := ksSvc.CreateOrPatch(ctx, h) if err != nil { return err @@ -757,7 +623,7 @@ func (r *PlacementAPIReconciler) ensureKeystoneEndpoint( instance.Namespace, ksEndptSpec, getServiceLabels(instance), - time.Duration(10)*time.Second, + r.RequeueTimeout, ) ctrlResult, err := ksEndpt.CreateOrPatch(ctx, h) if err != nil { @@ -1007,68 +873,24 @@ func (r *PlacementAPIReconciler) SetupWithManager(mgr ctrl.Manager) error { } func (r *PlacementAPIReconciler) findObjectsForSrc(ctx context.Context, src client.Object) []reconcile.Request { - requests := []reconcile.Request{} - - Log := r.GetLogger(context.Background()) - - for _, field := range allWatchFields { - crList := &placementv1.PlacementAPIList{} - listOps := &client.ListOptions{ - FieldSelector: fields.OneTermEqualSelector(field, src.GetName()), - Namespace: src.GetNamespace(), - } - err := r.List(ctx, crList, listOps) - if err != nil { - Log.Error(err, fmt.Sprintf("listing %s for field: %s - %s", crList.GroupVersionKind().Kind, field, src.GetNamespace())) - return requests - } - - for _, item := range crList.Items { - Log.Info(fmt.Sprintf("input source %s changed, reconcile: %s - %s", src.GetName(), item.GetName(), item.GetNamespace())) - - requests = append(requests, - reconcile.Request{ - NamespacedName: types.NamespacedName{ - Name: item.GetName(), - Namespace: item.GetNamespace(), - }, - }, - ) - } - } - - return requests + return internalcommon.FindObjectsForSrcByField( + ctx, + r.GetLogger(ctx), + r.Client, + src, + allWatchFields, + func() client.ObjectList { return &placementv1.PlacementAPIList{} }, + ) } func (r *PlacementAPIReconciler) findObjectForSrc(ctx context.Context, src client.Object) []reconcile.Request { - requests := []reconcile.Request{} - - Log := r.GetLogger(ctx) - - crList := &placementv1.PlacementAPIList{} - listOps := &client.ListOptions{ - Namespace: src.GetNamespace(), - } - err := r.List(ctx, crList, listOps) - if err != nil { - Log.Error(err, fmt.Sprintf("listing %s for namespace: %s", crList.GroupVersionKind().Kind, src.GetNamespace())) - return requests - } - - for _, item := range crList.Items { - Log.Info(fmt.Sprintf("input source %s changed, reconcile: %s - %s", src.GetName(), item.GetName(), item.GetNamespace())) - - requests = append(requests, - reconcile.Request{ - NamespacedName: types.NamespacedName{ - Name: item.GetName(), - Namespace: item.GetNamespace(), - }, - }, - ) - } - - return requests + return internalcommon.FindObjectsForSrcInNamespace( + ctx, + r.GetLogger(ctx), + r.Client, + src, + func() client.ObjectList { return &placementv1.PlacementAPIList{} }, + ) } func (r *PlacementAPIReconciler) reconcileDelete(ctx context.Context, instance *placementv1.PlacementAPI, helper *helper.Helper) (ctrl.Result, error) { @@ -1105,7 +927,7 @@ func (r *PlacementAPIReconciler) reconcileDelete(ctx context.Context, instance * if err == nil { if controllerutil.RemoveFinalizer(keystoneEndpoint, helper.GetFinalizer()) { - err = r.Update(ctx, keystoneEndpoint) + err = r.Client.Update(ctx, keystoneEndpoint) if err != nil && !k8s_errors.IsNotFound(err) { return ctrl.Result{}, err } @@ -1121,7 +943,7 @@ func (r *PlacementAPIReconciler) reconcileDelete(ctx context.Context, instance * if err == nil { if controllerutil.RemoveFinalizer(keystoneService, helper.GetFinalizer()) { - err = r.Update(ctx, keystoneService) + err = r.Client.Update(ctx, keystoneService) if err != nil && !k8s_errors.IsNotFound(err) { return ctrl.Result{}, err } @@ -1220,7 +1042,7 @@ func (r *PlacementAPIReconciler) ensureDbSync( jobDef, placementv1.DbSyncHash, instance.Spec.PreserveJobs, - time.Duration(5)*time.Second, + r.RequeueTimeout, dbSyncHash, ) ctrlResult, err := dbSyncjob.DoJob( @@ -1268,35 +1090,19 @@ func (r *PlacementAPIReconciler) ensureDeployment( // // Handle Topology // - topology, err := topologyv1.EnsureServiceTopology( + // If TopologyRef is present and ensureServiceTopology returned a valid + // topology object, set .Status.LastAppliedTopology to the referenced one + // and mark the condition as true + topology, err := internalcommon.EnsureTopology( ctx, h, - instance.Spec.TopologyRef, - instance.Status.LastAppliedTopology, + instance, instance.Name, + &instance.Status.Conditions, labels.GetLabelSelector(serviceLabels), ) if err != nil { - instance.Status.Conditions.Set(condition.FalseCondition( - condition.TopologyReadyCondition, - condition.ErrorReason, - condition.SeverityWarning, - condition.TopologyReadyErrorMessage, - err.Error())) - return ctrl.Result{}, fmt.Errorf("waiting for Topology requirements: %w", err) - } - - // If TopologyRef is present and ensureServiceTopology returned a valid - // topology object, set .Status.LastAppliedTopology to the referenced one - // and mark the condition as true - if instance.Spec.TopologyRef != nil { - // update the Status with the last retrieved Topology name - instance.Status.LastAppliedTopology = instance.Spec.TopologyRef - // update the TopologyRef associated condition - instance.Status.Conditions.MarkTrue(condition.TopologyReadyCondition, condition.TopologyReadyMessage) - } else { - // remove LastAppliedTopology from the .Status - instance.Status.LastAppliedTopology = nil + return ctrl.Result{}, err } // Define a new Deployment object @@ -1312,7 +1118,7 @@ func (r *PlacementAPIReconciler) ensureDeployment( depl := deployment.NewDeployment( deplDef, - time.Duration(5)*time.Second, + r.RequeueTimeout, ) ctrlResult, err := depl.CreateOrPatch(ctx, h) @@ -1471,8 +1277,7 @@ func (r *PlacementAPIReconciler) generateServiceConfigMaps( acSecretObj, _, err := secret.GetSecret(ctx, h, instance.Spec.Auth.ApplicationCredentialSecret, instance.Namespace) if err != nil { if k8s_errors.IsNotFound(err) { - h.GetLogger().Info("ApplicationCredential secret not found, waiting", "secret", instance.Spec.Auth.ApplicationCredentialSecret) - return fmt.Errorf("%w: %s", ErrACSecretNotFound, instance.Spec.Auth.ApplicationCredentialSecret) + return err } h.GetLogger().Error(err, "Failed to get ApplicationCredential secret", "secret", instance.Spec.Auth.ApplicationCredentialSecret) return err @@ -1481,7 +1286,7 @@ func (r *PlacementAPIReconciler) generateServiceConfigMaps( acSecretData, okSecret := acSecretObj.Data[keystonev1.ACSecretSecretKey] if !okID || len(acID) == 0 || !okSecret || len(acSecretData) == 0 { h.GetLogger().Info("ApplicationCredential secret missing required keys", "secret", instance.Spec.Auth.ApplicationCredentialSecret) - return fmt.Errorf("%w: %s", ErrACSecretMissingKeys, instance.Spec.Auth.ApplicationCredentialSecret) + return fmt.Errorf("%w: %s", internalcommon.ErrACSecretMissingKeys, instance.Spec.Auth.ApplicationCredentialSecret) } templateParameters["UseApplicationCredentials"] = true templateParameters["ACID"] = string(acID) @@ -1509,32 +1314,12 @@ func (r *PlacementAPIReconciler) generateServiceConfigMaps( "placement.conf": "placement/api/config/placement.conf", } - kind := instance.GetObjectKind().GroupVersionKind().Kind - cms := []util.Template{ - // ScriptsConfigMap - { - Name: fmt.Sprintf("%s-scripts", instance.Name), - Namespace: instance.Namespace, - Type: util.TemplateTypeScripts, - InstanceType: kind, - MultiTemplateDir: "placement/api", - Labels: cmLabels, - }, - // ConfigMap - { - Name: fmt.Sprintf("%s-config-data", instance.Name), - Namespace: instance.Namespace, - Type: util.TemplateTypeConfig, - InstanceType: kind, - MultiTemplateDir: "placement/api", - CustomData: customData, - ConfigOptions: templateParameters, - Labels: cmLabels, - AdditionalTemplate: extraTemplates, - CommonTemplates: []string{"ssl.conf"}, - }, - } - return secret.EnsureSecrets(ctx, h, instance, cms, envVars) + return internalcommon.GenerateConfigsWithScripts( + ctx, h, instance, envVars, templateParameters, customData, cmLabels, + extraTemplates, + []string{"ssl.conf"}, + "placement/api", + ) } // createHashOfInputHashes - creates a hash of hashes which gets added to the resources which requires a restart diff --git a/internal/nova/api/deployment.go b/internal/nova/api/deployment.go index 0bd3bd033..d7822374d 100644 --- a/internal/nova/api/deployment.go +++ b/internal/nova/api/deployment.go @@ -27,6 +27,7 @@ import ( "github.com/openstack-k8s-operators/lib-common/modules/common/service" "github.com/openstack-k8s-operators/lib-common/modules/common/tls" novav1 "github.com/openstack-k8s-operators/nova-operator/api/nova/v1beta1" + internalcommon "github.com/openstack-k8s-operators/nova-operator/internal/common" "github.com/openstack-k8s-operators/nova-operator/internal/nova" appsv1 "k8s.io/api/apps/v1" @@ -106,7 +107,7 @@ func StatefulSet( // create Volume and VolumeMounts volumes := []corev1.Volume{ - nova.GetConfigVolume(nova.GetServiceConfigSecretName(instance.Name)), + nova.GetConfigVolume(internalcommon.GetServiceConfigSecretName(instance.Name)), nova.GetLogVolume(), } volumeMounts := []corev1.VolumeMount{ diff --git a/internal/nova/common.go b/internal/nova/common.go index 900ef31a7..6751be98b 100644 --- a/internal/nova/common.go +++ b/internal/nova/common.go @@ -17,14 +17,13 @@ limitations under the License. package nova import ( - "fmt" - mariadbv1 "github.com/openstack-k8s-operators/mariadb-operator/api/v1beta1" + internalcommon "github.com/openstack-k8s-operators/nova-operator/internal/common" ) const ( // KollaServiceCommand - the command to start the service binary in the kolla container - KollaServiceCommand = "/usr/local/bin/kolla_start" + KollaServiceCommand = internalcommon.ServiceCommand // NovaUserID is the linux user ID used by Kolla for the nova user // in the service containers NovaUserID int64 = 42436 @@ -35,18 +34,6 @@ const ( ACConsumerFinalizer = "openstack.org/nova-ac-consumer" ) -// GetScriptSecretName returns the name of the Secret used for the -// db sync scripts -func GetScriptSecretName(crName string) string { - return fmt.Sprintf("%s-scripts", crName) -} - -// GetServiceConfigSecretName returns the name of the Secret used to -// store the service configuration files -func GetServiceConfigSecretName(crName string) string { - return fmt.Sprintf("%s-config-data", crName) -} - // DatabaseStatus - type DatabaseStatus int diff --git a/internal/nova/compute/deployment.go b/internal/nova/compute/deployment.go index d90b6a7c0..718da3eeb 100644 --- a/internal/nova/compute/deployment.go +++ b/internal/nova/compute/deployment.go @@ -22,6 +22,7 @@ import ( affinity "github.com/openstack-k8s-operators/lib-common/modules/common/affinity" env "github.com/openstack-k8s-operators/lib-common/modules/common/env" novav1 "github.com/openstack-k8s-operators/nova-operator/api/nova/v1beta1" + internalcommon "github.com/openstack-k8s-operators/nova-operator/internal/common" "github.com/openstack-k8s-operators/nova-operator/internal/nova" topologyv1 "github.com/openstack-k8s-operators/infra-operator/apis/topology/v1beta1" @@ -75,7 +76,7 @@ func StatefulSet( // create Volume and VolumeMounts volumes := []corev1.Volume{ - nova.GetConfigVolume(nova.GetServiceConfigSecretName(instance.Name)), + nova.GetConfigVolume(internalcommon.GetServiceConfigSecretName(instance.Name)), } volumeMounts := []corev1.VolumeMount{ nova.GetConfigVolumeMount(), diff --git a/internal/nova/conductor/dbpurge.go b/internal/nova/conductor/dbpurge.go index ddd38b3ef..660e690a1 100644 --- a/internal/nova/conductor/dbpurge.go +++ b/internal/nova/conductor/dbpurge.go @@ -13,6 +13,7 @@ import ( memcachedv1 "github.com/openstack-k8s-operators/infra-operator/apis/memcached/v1beta1" "github.com/openstack-k8s-operators/lib-common/modules/common/env" novav1 "github.com/openstack-k8s-operators/nova-operator/api/nova/v1beta1" + internalcommon "github.com/openstack-k8s-operators/nova-operator/internal/common" "github.com/openstack-k8s-operators/nova-operator/internal/nova" ) @@ -35,8 +36,8 @@ func DBPurgeCronJob( env := env.MergeEnvs([]corev1.EnvVar{}, envVars) volumes := []corev1.Volume{ - nova.GetConfigVolume(nova.GetServiceConfigSecretName(instance.Name)), - nova.GetScriptVolume(nova.GetScriptSecretName(instance.Name)), + nova.GetConfigVolume(internalcommon.GetServiceConfigSecretName(instance.Name)), + nova.GetScriptVolume(internalcommon.GetScriptSecretName(instance.Name)), } volumeMounts := []corev1.VolumeMount{ nova.GetConfigVolumeMount(), diff --git a/internal/nova/conductor/dbsync.go b/internal/nova/conductor/dbsync.go index 856bc8f0f..e682c1d31 100644 --- a/internal/nova/conductor/dbsync.go +++ b/internal/nova/conductor/dbsync.go @@ -18,6 +18,7 @@ package novaconductor import ( novav1 "github.com/openstack-k8s-operators/nova-operator/api/nova/v1beta1" + internalcommon "github.com/openstack-k8s-operators/nova-operator/internal/common" "github.com/openstack-k8s-operators/nova-operator/internal/nova" env "github.com/openstack-k8s-operators/lib-common/modules/common/env" @@ -48,8 +49,8 @@ func CellDBSyncJob( // create Volume and VolumeMounts volumes := []corev1.Volume{ - nova.GetConfigVolume(nova.GetServiceConfigSecretName(instance.Name)), - nova.GetScriptVolume(nova.GetScriptSecretName(instance.Name)), + nova.GetConfigVolume(internalcommon.GetServiceConfigSecretName(instance.Name)), + nova.GetScriptVolume(internalcommon.GetScriptSecretName(instance.Name)), } volumeMounts := []corev1.VolumeMount{ nova.GetConfigVolumeMount(), diff --git a/internal/nova/conductor/deployment.go b/internal/nova/conductor/deployment.go index aeedcad59..32a2e758c 100644 --- a/internal/nova/conductor/deployment.go +++ b/internal/nova/conductor/deployment.go @@ -23,6 +23,7 @@ import ( affinity "github.com/openstack-k8s-operators/lib-common/modules/common/affinity" env "github.com/openstack-k8s-operators/lib-common/modules/common/env" novav1 "github.com/openstack-k8s-operators/nova-operator/api/nova/v1beta1" + internalcommon "github.com/openstack-k8s-operators/nova-operator/internal/common" "github.com/openstack-k8s-operators/nova-operator/internal/nova" appsv1 "k8s.io/api/apps/v1" @@ -82,7 +83,7 @@ func StatefulSet( // create Volume and VolumeMounts volumes := []corev1.Volume{ - nova.GetConfigVolume(nova.GetServiceConfigSecretName(instance.Name)), + nova.GetConfigVolume(internalcommon.GetServiceConfigSecretName(instance.Name)), } volumeMounts := []corev1.VolumeMount{ nova.GetConfigVolumeMount(), diff --git a/internal/nova/metadata/deployment.go b/internal/nova/metadata/deployment.go index 3c914fac2..a60366a0a 100644 --- a/internal/nova/metadata/deployment.go +++ b/internal/nova/metadata/deployment.go @@ -25,6 +25,7 @@ import ( affinity "github.com/openstack-k8s-operators/lib-common/modules/common/affinity" env "github.com/openstack-k8s-operators/lib-common/modules/common/env" novav1 "github.com/openstack-k8s-operators/nova-operator/api/nova/v1beta1" + internalcommon "github.com/openstack-k8s-operators/nova-operator/internal/common" "github.com/openstack-k8s-operators/nova-operator/internal/nova" appsv1 "k8s.io/api/apps/v1" @@ -104,7 +105,7 @@ func StatefulSet( // create Volume and VolumeMounts volumes := []corev1.Volume{ - nova.GetConfigVolume(nova.GetServiceConfigSecretName(instance.Name)), + nova.GetConfigVolume(internalcommon.GetServiceConfigSecretName(instance.Name)), nova.GetLogVolume(), } volumeMounts := []corev1.VolumeMount{ diff --git a/internal/nova/novncproxy/deployment.go b/internal/nova/novncproxy/deployment.go index 9066de09a..fd8fe9d4e 100644 --- a/internal/nova/novncproxy/deployment.go +++ b/internal/nova/novncproxy/deployment.go @@ -23,6 +23,7 @@ import ( affinity "github.com/openstack-k8s-operators/lib-common/modules/common/affinity" env "github.com/openstack-k8s-operators/lib-common/modules/common/env" novav1 "github.com/openstack-k8s-operators/nova-operator/api/nova/v1beta1" + internalcommon "github.com/openstack-k8s-operators/nova-operator/internal/common" "github.com/openstack-k8s-operators/nova-operator/internal/nova" appsv1 "k8s.io/api/apps/v1" @@ -90,7 +91,7 @@ func StatefulSet( // create Volume and VolumeMounts volumes := []corev1.Volume{ - nova.GetConfigVolume(nova.GetServiceConfigSecretName(instance.Name)), + nova.GetConfigVolume(internalcommon.GetServiceConfigSecretName(instance.Name)), } volumeMounts := []corev1.VolumeMount{ nova.GetConfigVolumeMount(), diff --git a/internal/nova/scheduler/deployment.go b/internal/nova/scheduler/deployment.go index aa3bfc5d1..47f1271ec 100644 --- a/internal/nova/scheduler/deployment.go +++ b/internal/nova/scheduler/deployment.go @@ -22,6 +22,7 @@ import ( affinity "github.com/openstack-k8s-operators/lib-common/modules/common/affinity" env "github.com/openstack-k8s-operators/lib-common/modules/common/env" novav1 "github.com/openstack-k8s-operators/nova-operator/api/nova/v1beta1" + internalcommon "github.com/openstack-k8s-operators/nova-operator/internal/common" "github.com/openstack-k8s-operators/nova-operator/internal/nova" memcachedv1 "github.com/openstack-k8s-operators/infra-operator/apis/memcached/v1beta1" @@ -87,7 +88,7 @@ func StatefulSet( // create Volume and VolumeMounts volumes := []corev1.Volume{ - nova.GetConfigVolume(nova.GetServiceConfigSecretName(instance.Name)), + nova.GetConfigVolume(internalcommon.GetServiceConfigSecretName(instance.Name)), } volumeMounts := []corev1.VolumeMount{ nova.GetConfigVolumeMount(), diff --git a/internal/placement/const.go b/internal/placement/const.go index b678734d1..ec0f5a7f0 100644 --- a/internal/placement/const.go +++ b/internal/placement/const.go @@ -16,6 +16,10 @@ limitations under the License. // Package placement provides constants and utilities for managing OpenStack Placement service package placement +import ( + internalcommon "github.com/openstack-k8s-operators/nova-operator/internal/common" +) + const ( // ServiceName - ServiceName = "placement" @@ -31,7 +35,7 @@ const ( PlacementInternalPort int32 = 8778 // KollaServiceCommand is the command used to start the placement service in Kolla containers - KollaServiceCommand = "/usr/local/bin/kolla_start" + KollaServiceCommand = internalcommon.ServiceCommand // PlacementUserID is the linux user ID used by Kolla for the placement // user in the service containers diff --git a/internal/placement/volumes.go b/internal/placement/volumes.go index fa60ef15e..84b9a37ff 100644 --- a/internal/placement/volumes.go +++ b/internal/placement/volumes.go @@ -16,6 +16,8 @@ limitations under the License. package placement import ( + internalcommon "github.com/openstack-k8s-operators/nova-operator/internal/common" + corev1 "k8s.io/api/core/v1" ) @@ -30,7 +32,7 @@ func getVolumes(name string) []corev1.Volume { VolumeSource: corev1.VolumeSource{ Secret: &corev1.SecretVolumeSource{ DefaultMode: &scriptsVolumeDefaultMode, - SecretName: name + "-scripts", + SecretName: internalcommon.GetScriptSecretName(name), }, }, }, @@ -39,7 +41,7 @@ func getVolumes(name string) []corev1.Volume { VolumeSource: corev1.VolumeSource{ Secret: &corev1.SecretVolumeSource{ DefaultMode: &configMode, - SecretName: name + "-config-data", + SecretName: internalcommon.GetServiceConfigSecretName(name), }, }, }, diff --git a/internal/webhook/nova/v1beta1/nova_webhook.go b/internal/webhook/nova/v1beta1/nova_webhook.go index 204d7304a..d936410ee 100644 --- a/internal/webhook/nova/v1beta1/nova_webhook.go +++ b/internal/webhook/nova/v1beta1/nova_webhook.go @@ -19,7 +19,6 @@ package v1beta1 import ( "context" - "errors" "fmt" "k8s.io/apimachinery/pkg/runtime" @@ -29,16 +28,13 @@ import ( "sigs.k8s.io/controller-runtime/pkg/webhook/admission" novav1beta1 "github.com/openstack-k8s-operators/nova-operator/api/nova/v1beta1" + internalcommon "github.com/openstack-k8s-operators/nova-operator/internal/common" ) // nolint:unused // log is for logging in this package. var novalog = logf.Log.WithName("nova-resource") -var ( - errUnexpectedObjectType = errors.New("unexpected object type") -) - // SetupNovaWebhookWithManager registers the webhook for Nova in the manager. func SetupNovaWebhookWithManager(mgr ctrl.Manager) error { return ctrl.NewWebhookManagedBy(mgr).For(&novav1beta1.Nova{}). @@ -67,7 +63,7 @@ func (d *NovaCustomDefaulter) Default(_ context.Context, obj runtime.Object) err nova, ok := obj.(*novav1beta1.Nova) if !ok { - return fmt.Errorf("%w: expected an Nova object but got %T", errUnexpectedObjectType, obj) + return fmt.Errorf("expected a Nova object but got %T: %w", obj, internalcommon.ErrUnexpectedObjectType) } novalog.Info("Defaulting for Nova", "name", nova.GetName()) @@ -96,7 +92,7 @@ var _ webhook.CustomValidator = &NovaCustomValidator{} func (v *NovaCustomValidator) ValidateCreate(_ context.Context, obj runtime.Object) (admission.Warnings, error) { nova, ok := obj.(*novav1beta1.Nova) if !ok { - return nil, fmt.Errorf("%w: expected a Nova object but got %T", errUnexpectedObjectType, obj) + return nil, fmt.Errorf("expected a Nova object but got %T: %w", obj, internalcommon.ErrUnexpectedObjectType) } novalog.Info("Validation for Nova upon creation", "name", nova.GetName()) @@ -107,7 +103,7 @@ func (v *NovaCustomValidator) ValidateCreate(_ context.Context, obj runtime.Obje func (v *NovaCustomValidator) ValidateUpdate(_ context.Context, oldObj, newObj runtime.Object) (admission.Warnings, error) { nova, ok := newObj.(*novav1beta1.Nova) if !ok { - return nil, fmt.Errorf("%w: expected a Nova object for the newObj but got %T", errUnexpectedObjectType, newObj) + return nil, fmt.Errorf("expected a Nova object for the newObj but got %T: %w", newObj, internalcommon.ErrUnexpectedObjectType) } novalog.Info("Validation for Nova upon update", "name", nova.GetName()) @@ -118,7 +114,7 @@ func (v *NovaCustomValidator) ValidateUpdate(_ context.Context, oldObj, newObj r func (v *NovaCustomValidator) ValidateDelete(_ context.Context, obj runtime.Object) (admission.Warnings, error) { nova, ok := obj.(*novav1beta1.Nova) if !ok { - return nil, fmt.Errorf("%w: expected a Nova object but got %T", errUnexpectedObjectType, obj) + return nil, fmt.Errorf("expected a Nova object but got %T: %w", obj, internalcommon.ErrUnexpectedObjectType) } novalog.Info("Validation for Nova upon deletion", "name", nova.GetName()) diff --git a/internal/webhook/nova/v1beta1/novaapi_webhook.go b/internal/webhook/nova/v1beta1/novaapi_webhook.go index fae16fda4..09ac91d7b 100644 --- a/internal/webhook/nova/v1beta1/novaapi_webhook.go +++ b/internal/webhook/nova/v1beta1/novaapi_webhook.go @@ -18,7 +18,6 @@ package v1beta1 import ( "context" - "errors" "fmt" "k8s.io/apimachinery/pkg/runtime" @@ -28,14 +27,13 @@ import ( "sigs.k8s.io/controller-runtime/pkg/webhook/admission" novav1beta1 "github.com/openstack-k8s-operators/nova-operator/api/nova/v1beta1" + internalcommon "github.com/openstack-k8s-operators/nova-operator/internal/common" ) // nolint:unused // log is for logging in this package. var novaapilog = logf.Log.WithName("novaapi-resource") -var errExpectedNovaAPIObject = errors.New("expected a NovaAPI object") - // SetupNovaAPIWebhookWithManager registers the webhook for NovaAPI in the manager. func SetupNovaAPIWebhookWithManager(mgr ctrl.Manager) error { return ctrl.NewWebhookManagedBy(mgr).For(&novav1beta1.NovaAPI{}). @@ -64,7 +62,7 @@ func (d *NovaAPICustomDefaulter) Default(_ context.Context, obj runtime.Object) novaapi, ok := obj.(*novav1beta1.NovaAPI) if !ok { - return fmt.Errorf("%w but got %T", errExpectedNovaAPIObject, obj) + return fmt.Errorf("expected a NovaAPI object but got %T: %w", obj, internalcommon.ErrUnexpectedObjectType) } novaapilog.Info("Defaulting for NovaAPI", "name", novaapi.GetName()) @@ -93,7 +91,7 @@ var _ webhook.CustomValidator = &NovaAPICustomValidator{} func (v *NovaAPICustomValidator) ValidateCreate(_ context.Context, obj runtime.Object) (admission.Warnings, error) { novaapi, ok := obj.(*novav1beta1.NovaAPI) if !ok { - return nil, fmt.Errorf("%w but got %T", errExpectedNovaAPIObject, obj) + return nil, fmt.Errorf("expected a NovaAPI object but got %T: %w", obj, internalcommon.ErrUnexpectedObjectType) } novaapilog.Info("Validation for NovaAPI upon creation", "name", novaapi.GetName()) @@ -104,7 +102,7 @@ func (v *NovaAPICustomValidator) ValidateCreate(_ context.Context, obj runtime.O func (v *NovaAPICustomValidator) ValidateUpdate(_ context.Context, oldObj, newObj runtime.Object) (admission.Warnings, error) { novaapi, ok := newObj.(*novav1beta1.NovaAPI) if !ok { - return nil, fmt.Errorf("%w for the newObj but got %T", errExpectedNovaAPIObject, newObj) + return nil, fmt.Errorf("expected a NovaAPI object for the newObj but got %T: %w", newObj, internalcommon.ErrUnexpectedObjectType) } novaapilog.Info("Validation for NovaAPI upon update", "name", novaapi.GetName()) @@ -115,7 +113,7 @@ func (v *NovaAPICustomValidator) ValidateUpdate(_ context.Context, oldObj, newOb func (v *NovaAPICustomValidator) ValidateDelete(_ context.Context, obj runtime.Object) (admission.Warnings, error) { novaapi, ok := obj.(*novav1beta1.NovaAPI) if !ok { - return nil, fmt.Errorf("%w but got %T", errExpectedNovaAPIObject, obj) + return nil, fmt.Errorf("expected a NovaAPI object but got %T: %w", obj, internalcommon.ErrUnexpectedObjectType) } novaapilog.Info("Validation for NovaAPI upon deletion", "name", novaapi.GetName()) diff --git a/internal/webhook/nova/v1beta1/novacell_webhook.go b/internal/webhook/nova/v1beta1/novacell_webhook.go index 002b1bac3..d61a8ec2d 100644 --- a/internal/webhook/nova/v1beta1/novacell_webhook.go +++ b/internal/webhook/nova/v1beta1/novacell_webhook.go @@ -18,7 +18,6 @@ package v1beta1 import ( "context" - "errors" "fmt" "k8s.io/apimachinery/pkg/runtime" @@ -28,14 +27,13 @@ import ( "sigs.k8s.io/controller-runtime/pkg/webhook/admission" novav1beta1 "github.com/openstack-k8s-operators/nova-operator/api/nova/v1beta1" + internalcommon "github.com/openstack-k8s-operators/nova-operator/internal/common" ) // nolint:unused // log is for logging in this package. var novacelllog = logf.Log.WithName("novacell-resource") -var errExpectedNovaCellObject = errors.New("expected a NovaCell object") - // SetupNovaCellWebhookWithManager registers the webhook for NovaCell in the manager. func SetupNovaCellWebhookWithManager(mgr ctrl.Manager) error { return ctrl.NewWebhookManagedBy(mgr).For(&novav1beta1.NovaCell{}). @@ -64,7 +62,7 @@ func (d *NovaCellCustomDefaulter) Default(_ context.Context, obj runtime.Object) novacell, ok := obj.(*novav1beta1.NovaCell) if !ok { - return fmt.Errorf("%w but got %T", errExpectedNovaCellObject, obj) + return fmt.Errorf("expected a NovaCell object but got %T: %w", obj, internalcommon.ErrUnexpectedObjectType) } novacelllog.Info("Defaulting for NovaCell", "name", novacell.GetName()) @@ -93,7 +91,7 @@ var _ webhook.CustomValidator = &NovaCellCustomValidator{} func (v *NovaCellCustomValidator) ValidateCreate(_ context.Context, obj runtime.Object) (admission.Warnings, error) { novacell, ok := obj.(*novav1beta1.NovaCell) if !ok { - return nil, fmt.Errorf("%w but got %T", errExpectedNovaCellObject, obj) + return nil, fmt.Errorf("expected a NovaCell object but got %T: %w", obj, internalcommon.ErrUnexpectedObjectType) } novacelllog.Info("Validation for NovaCell upon creation", "name", novacell.GetName()) @@ -104,7 +102,7 @@ func (v *NovaCellCustomValidator) ValidateCreate(_ context.Context, obj runtime. func (v *NovaCellCustomValidator) ValidateUpdate(_ context.Context, oldObj, newObj runtime.Object) (admission.Warnings, error) { novacell, ok := newObj.(*novav1beta1.NovaCell) if !ok { - return nil, fmt.Errorf("%w for the newObj but got %T", errExpectedNovaCellObject, newObj) + return nil, fmt.Errorf("expected a NovaCell object for the newObj but got %T: %w", newObj, internalcommon.ErrUnexpectedObjectType) } novacelllog.Info("Validation for NovaCell upon update", "name", novacell.GetName()) @@ -115,7 +113,7 @@ func (v *NovaCellCustomValidator) ValidateUpdate(_ context.Context, oldObj, newO func (v *NovaCellCustomValidator) ValidateDelete(_ context.Context, obj runtime.Object) (admission.Warnings, error) { novacell, ok := obj.(*novav1beta1.NovaCell) if !ok { - return nil, fmt.Errorf("%w but got %T", errExpectedNovaCellObject, obj) + return nil, fmt.Errorf("expected a NovaCell object but got %T: %w", obj, internalcommon.ErrUnexpectedObjectType) } novacelllog.Info("Validation for NovaCell upon deletion", "name", novacell.GetName()) diff --git a/internal/webhook/nova/v1beta1/novacompute_webhook.go b/internal/webhook/nova/v1beta1/novacompute_webhook.go index 91b0870e7..e42e52ded 100644 --- a/internal/webhook/nova/v1beta1/novacompute_webhook.go +++ b/internal/webhook/nova/v1beta1/novacompute_webhook.go @@ -18,7 +18,6 @@ package v1beta1 import ( "context" - "errors" "fmt" "k8s.io/apimachinery/pkg/runtime" @@ -28,14 +27,13 @@ import ( "sigs.k8s.io/controller-runtime/pkg/webhook/admission" novav1beta1 "github.com/openstack-k8s-operators/nova-operator/api/nova/v1beta1" + internalcommon "github.com/openstack-k8s-operators/nova-operator/internal/common" ) // nolint:unused // log is for logging in this package. var novacomputelog = logf.Log.WithName("novacompute-resource") -var errExpectedNovaComputeObject = errors.New("expected a NovaCompute object") - // SetupNovaComputeWebhookWithManager registers the webhook for NovaCompute in the manager. func SetupNovaComputeWebhookWithManager(mgr ctrl.Manager) error { return ctrl.NewWebhookManagedBy(mgr).For(&novav1beta1.NovaCompute{}). @@ -64,7 +62,7 @@ func (d *NovaComputeCustomDefaulter) Default(_ context.Context, obj runtime.Obje novacompute, ok := obj.(*novav1beta1.NovaCompute) if !ok { - return fmt.Errorf("%w but got %T", errExpectedNovaComputeObject, obj) + return fmt.Errorf("expected a NovaCompute object but got %T: %w", obj, internalcommon.ErrUnexpectedObjectType) } novacomputelog.Info("Defaulting for NovaCompute", "name", novacompute.GetName()) @@ -93,7 +91,7 @@ var _ webhook.CustomValidator = &NovaComputeCustomValidator{} func (v *NovaComputeCustomValidator) ValidateCreate(_ context.Context, obj runtime.Object) (admission.Warnings, error) { novacompute, ok := obj.(*novav1beta1.NovaCompute) if !ok { - return nil, fmt.Errorf("%w but got %T", errExpectedNovaComputeObject, obj) + return nil, fmt.Errorf("expected a NovaCompute object but got %T: %w", obj, internalcommon.ErrUnexpectedObjectType) } novacomputelog.Info("Validation for NovaCompute upon creation", "name", novacompute.GetName()) @@ -104,7 +102,7 @@ func (v *NovaComputeCustomValidator) ValidateCreate(_ context.Context, obj runti func (v *NovaComputeCustomValidator) ValidateUpdate(_ context.Context, oldObj, newObj runtime.Object) (admission.Warnings, error) { novacompute, ok := newObj.(*novav1beta1.NovaCompute) if !ok { - return nil, fmt.Errorf("%w for the newObj but got %T", errExpectedNovaComputeObject, newObj) + return nil, fmt.Errorf("expected a NovaCompute object for the newObj but got %T: %w", newObj, internalcommon.ErrUnexpectedObjectType) } novacomputelog.Info("Validation for NovaCompute upon update", "name", novacompute.GetName()) @@ -115,7 +113,7 @@ func (v *NovaComputeCustomValidator) ValidateUpdate(_ context.Context, oldObj, n func (v *NovaComputeCustomValidator) ValidateDelete(_ context.Context, obj runtime.Object) (admission.Warnings, error) { novacompute, ok := obj.(*novav1beta1.NovaCompute) if !ok { - return nil, fmt.Errorf("%w but got %T", errExpectedNovaComputeObject, obj) + return nil, fmt.Errorf("expected a NovaCompute object but got %T: %w", obj, internalcommon.ErrUnexpectedObjectType) } novacomputelog.Info("Validation for NovaCompute upon deletion", "name", novacompute.GetName()) diff --git a/internal/webhook/nova/v1beta1/novaconductor_webhook.go b/internal/webhook/nova/v1beta1/novaconductor_webhook.go index 6618a8170..d122bfbd4 100644 --- a/internal/webhook/nova/v1beta1/novaconductor_webhook.go +++ b/internal/webhook/nova/v1beta1/novaconductor_webhook.go @@ -18,7 +18,6 @@ package v1beta1 import ( "context" - "errors" "fmt" "k8s.io/apimachinery/pkg/runtime" @@ -28,14 +27,13 @@ import ( "sigs.k8s.io/controller-runtime/pkg/webhook/admission" novav1beta1 "github.com/openstack-k8s-operators/nova-operator/api/nova/v1beta1" + internalcommon "github.com/openstack-k8s-operators/nova-operator/internal/common" ) // nolint:unused // log is for logging in this package. var novaconductorlog = logf.Log.WithName("novaconductor-resource") -var errExpectedNovaConductorObject = errors.New("expected a NovaConductor object") - // SetupNovaConductorWebhookWithManager registers the webhook for NovaConductor in the manager. func SetupNovaConductorWebhookWithManager(mgr ctrl.Manager) error { return ctrl.NewWebhookManagedBy(mgr).For(&novav1beta1.NovaConductor{}). @@ -64,7 +62,7 @@ func (d *NovaConductorCustomDefaulter) Default(_ context.Context, obj runtime.Ob novaconductor, ok := obj.(*novav1beta1.NovaConductor) if !ok { - return fmt.Errorf("%w but got %T", errExpectedNovaConductorObject, obj) + return fmt.Errorf("expected a NovaConductor object but got %T: %w", obj, internalcommon.ErrUnexpectedObjectType) } novaconductorlog.Info("Defaulting for NovaConductor", "name", novaconductor.GetName()) @@ -93,7 +91,7 @@ var _ webhook.CustomValidator = &NovaConductorCustomValidator{} func (v *NovaConductorCustomValidator) ValidateCreate(_ context.Context, obj runtime.Object) (admission.Warnings, error) { novaconductor, ok := obj.(*novav1beta1.NovaConductor) if !ok { - return nil, fmt.Errorf("%w but got %T", errExpectedNovaConductorObject, obj) + return nil, fmt.Errorf("expected a NovaConductor object but got %T: %w", obj, internalcommon.ErrUnexpectedObjectType) } novaconductorlog.Info("Validation for NovaConductor upon creation", "name", novaconductor.GetName()) @@ -104,7 +102,7 @@ func (v *NovaConductorCustomValidator) ValidateCreate(_ context.Context, obj run func (v *NovaConductorCustomValidator) ValidateUpdate(_ context.Context, oldObj, newObj runtime.Object) (admission.Warnings, error) { novaconductor, ok := newObj.(*novav1beta1.NovaConductor) if !ok { - return nil, fmt.Errorf("%w for the newObj but got %T", errExpectedNovaConductorObject, newObj) + return nil, fmt.Errorf("expected a NovaConductor object for the newObj but got %T: %w", newObj, internalcommon.ErrUnexpectedObjectType) } novaconductorlog.Info("Validation for NovaConductor upon update", "name", novaconductor.GetName()) @@ -115,7 +113,7 @@ func (v *NovaConductorCustomValidator) ValidateUpdate(_ context.Context, oldObj, func (v *NovaConductorCustomValidator) ValidateDelete(_ context.Context, obj runtime.Object) (admission.Warnings, error) { novaconductor, ok := obj.(*novav1beta1.NovaConductor) if !ok { - return nil, fmt.Errorf("%w but got %T", errExpectedNovaConductorObject, obj) + return nil, fmt.Errorf("expected a NovaConductor object but got %T: %w", obj, internalcommon.ErrUnexpectedObjectType) } novaconductorlog.Info("Validation for NovaConductor upon deletion", "name", novaconductor.GetName()) diff --git a/internal/webhook/nova/v1beta1/novametadata_webhook.go b/internal/webhook/nova/v1beta1/novametadata_webhook.go index daca00809..7d595bfd2 100644 --- a/internal/webhook/nova/v1beta1/novametadata_webhook.go +++ b/internal/webhook/nova/v1beta1/novametadata_webhook.go @@ -18,7 +18,6 @@ package v1beta1 import ( "context" - "errors" "fmt" "k8s.io/apimachinery/pkg/runtime" @@ -28,14 +27,13 @@ import ( "sigs.k8s.io/controller-runtime/pkg/webhook/admission" novav1beta1 "github.com/openstack-k8s-operators/nova-operator/api/nova/v1beta1" + internalcommon "github.com/openstack-k8s-operators/nova-operator/internal/common" ) // nolint:unused // log is for logging in this package. var novametadatalog = logf.Log.WithName("novametadata-resource") -var errExpectedNovaMetadataObject = errors.New("expected a NovaMetadata object") - // SetupNovaMetadataWebhookWithManager registers the webhook for NovaMetadata in the manager. func SetupNovaMetadataWebhookWithManager(mgr ctrl.Manager) error { return ctrl.NewWebhookManagedBy(mgr).For(&novav1beta1.NovaMetadata{}). @@ -64,7 +62,7 @@ func (d *NovaMetadataCustomDefaulter) Default(_ context.Context, obj runtime.Obj novametadata, ok := obj.(*novav1beta1.NovaMetadata) if !ok { - return fmt.Errorf("%w but got %T", errExpectedNovaMetadataObject, obj) + return fmt.Errorf("expected a NovaMetadata object but got %T: %w", obj, internalcommon.ErrUnexpectedObjectType) } novametadatalog.Info("Defaulting for NovaMetadata", "name", novametadata.GetName()) @@ -93,7 +91,7 @@ var _ webhook.CustomValidator = &NovaMetadataCustomValidator{} func (v *NovaMetadataCustomValidator) ValidateCreate(_ context.Context, obj runtime.Object) (admission.Warnings, error) { novametadata, ok := obj.(*novav1beta1.NovaMetadata) if !ok { - return nil, fmt.Errorf("%w but got %T", errExpectedNovaMetadataObject, obj) + return nil, fmt.Errorf("expected a NovaMetadata object but got %T: %w", obj, internalcommon.ErrUnexpectedObjectType) } novametadatalog.Info("Validation for NovaMetadata upon creation", "name", novametadata.GetName()) @@ -104,7 +102,7 @@ func (v *NovaMetadataCustomValidator) ValidateCreate(_ context.Context, obj runt func (v *NovaMetadataCustomValidator) ValidateUpdate(_ context.Context, oldObj, newObj runtime.Object) (admission.Warnings, error) { novametadata, ok := newObj.(*novav1beta1.NovaMetadata) if !ok { - return nil, fmt.Errorf("%w for the newObj but got %T", errExpectedNovaMetadataObject, newObj) + return nil, fmt.Errorf("expected a NovaMetadata object for the newObj but got %T: %w", newObj, internalcommon.ErrUnexpectedObjectType) } novametadatalog.Info("Validation for NovaMetadata upon update", "name", novametadata.GetName()) @@ -115,7 +113,7 @@ func (v *NovaMetadataCustomValidator) ValidateUpdate(_ context.Context, oldObj, func (v *NovaMetadataCustomValidator) ValidateDelete(_ context.Context, obj runtime.Object) (admission.Warnings, error) { novametadata, ok := obj.(*novav1beta1.NovaMetadata) if !ok { - return nil, fmt.Errorf("%w but got %T", errExpectedNovaMetadataObject, obj) + return nil, fmt.Errorf("expected a NovaMetadata object but got %T: %w", obj, internalcommon.ErrUnexpectedObjectType) } novametadatalog.Info("Validation for NovaMetadata upon deletion", "name", novametadata.GetName()) diff --git a/internal/webhook/nova/v1beta1/novanovncproxy_webhook.go b/internal/webhook/nova/v1beta1/novanovncproxy_webhook.go index 6b058100b..1825eebc3 100644 --- a/internal/webhook/nova/v1beta1/novanovncproxy_webhook.go +++ b/internal/webhook/nova/v1beta1/novanovncproxy_webhook.go @@ -18,7 +18,6 @@ package v1beta1 import ( "context" - "errors" "fmt" "k8s.io/apimachinery/pkg/runtime" @@ -28,14 +27,13 @@ import ( "sigs.k8s.io/controller-runtime/pkg/webhook/admission" novav1beta1 "github.com/openstack-k8s-operators/nova-operator/api/nova/v1beta1" + internalcommon "github.com/openstack-k8s-operators/nova-operator/internal/common" ) // nolint:unused // log is for logging in this package. var novanovncproxylog = logf.Log.WithName("novanovncproxy-resource") -var errExpectedNovaNoVNCProxyObject = errors.New("expected a NovaNoVNCProxy object") - // SetupNovaNoVNCProxyWebhookWithManager registers the webhook for NovaNoVNCProxy in the manager. func SetupNovaNoVNCProxyWebhookWithManager(mgr ctrl.Manager) error { return ctrl.NewWebhookManagedBy(mgr).For(&novav1beta1.NovaNoVNCProxy{}). @@ -64,7 +62,7 @@ func (d *NovaNoVNCProxyCustomDefaulter) Default(_ context.Context, obj runtime.O novanovncproxy, ok := obj.(*novav1beta1.NovaNoVNCProxy) if !ok { - return fmt.Errorf("%w but got %T", errExpectedNovaNoVNCProxyObject, obj) + return fmt.Errorf("expected a NovaNoVNCProxy object but got %T: %w", obj, internalcommon.ErrUnexpectedObjectType) } novanovncproxylog.Info("Defaulting for NovaNoVNCProxy", "name", novanovncproxy.GetName()) @@ -93,7 +91,7 @@ var _ webhook.CustomValidator = &NovaNoVNCProxyCustomValidator{} func (v *NovaNoVNCProxyCustomValidator) ValidateCreate(_ context.Context, obj runtime.Object) (admission.Warnings, error) { novanovncproxy, ok := obj.(*novav1beta1.NovaNoVNCProxy) if !ok { - return nil, fmt.Errorf("%w but got %T", errExpectedNovaNoVNCProxyObject, obj) + return nil, fmt.Errorf("expected a NovaNoVNCProxy object but got %T: %w", obj, internalcommon.ErrUnexpectedObjectType) } novanovncproxylog.Info("Validation for NovaNoVNCProxy upon creation", "name", novanovncproxy.GetName()) @@ -104,7 +102,7 @@ func (v *NovaNoVNCProxyCustomValidator) ValidateCreate(_ context.Context, obj ru func (v *NovaNoVNCProxyCustomValidator) ValidateUpdate(_ context.Context, oldObj, newObj runtime.Object) (admission.Warnings, error) { novanovncproxy, ok := newObj.(*novav1beta1.NovaNoVNCProxy) if !ok { - return nil, fmt.Errorf("%w for the newObj but got %T", errExpectedNovaNoVNCProxyObject, newObj) + return nil, fmt.Errorf("expected a NovaNoVNCProxy object for the newObj but got %T: %w", newObj, internalcommon.ErrUnexpectedObjectType) } novanovncproxylog.Info("Validation for NovaNoVNCProxy upon update", "name", novanovncproxy.GetName()) @@ -115,7 +113,7 @@ func (v *NovaNoVNCProxyCustomValidator) ValidateUpdate(_ context.Context, oldObj func (v *NovaNoVNCProxyCustomValidator) ValidateDelete(_ context.Context, obj runtime.Object) (admission.Warnings, error) { novanovncproxy, ok := obj.(*novav1beta1.NovaNoVNCProxy) if !ok { - return nil, fmt.Errorf("%w but got %T", errExpectedNovaNoVNCProxyObject, obj) + return nil, fmt.Errorf("expected a NovaNoVNCProxy object but got %T: %w", obj, internalcommon.ErrUnexpectedObjectType) } novanovncproxylog.Info("Validation for NovaNoVNCProxy upon deletion", "name", novanovncproxy.GetName()) diff --git a/internal/webhook/nova/v1beta1/novascheduler_webhook.go b/internal/webhook/nova/v1beta1/novascheduler_webhook.go index 3b456c93b..4249740a9 100644 --- a/internal/webhook/nova/v1beta1/novascheduler_webhook.go +++ b/internal/webhook/nova/v1beta1/novascheduler_webhook.go @@ -18,7 +18,6 @@ package v1beta1 import ( "context" - "errors" "fmt" "k8s.io/apimachinery/pkg/runtime" @@ -28,14 +27,13 @@ import ( "sigs.k8s.io/controller-runtime/pkg/webhook/admission" novav1beta1 "github.com/openstack-k8s-operators/nova-operator/api/nova/v1beta1" + internalcommon "github.com/openstack-k8s-operators/nova-operator/internal/common" ) // nolint:unused // log is for logging in this package. var novaschedulerlog = logf.Log.WithName("novascheduler-resource") -var errExpectedNovaSchedulerObject = errors.New("expected a NovaScheduler object") - // SetupNovaSchedulerWebhookWithManager registers the webhook for NovaScheduler in the manager. func SetupNovaSchedulerWebhookWithManager(mgr ctrl.Manager) error { return ctrl.NewWebhookManagedBy(mgr).For(&novav1beta1.NovaScheduler{}). @@ -64,7 +62,7 @@ func (d *NovaSchedulerCustomDefaulter) Default(_ context.Context, obj runtime.Ob novascheduler, ok := obj.(*novav1beta1.NovaScheduler) if !ok { - return fmt.Errorf("%w but got %T", errExpectedNovaSchedulerObject, obj) + return fmt.Errorf("expected a NovaScheduler object but got %T: %w", obj, internalcommon.ErrUnexpectedObjectType) } novaschedulerlog.Info("Defaulting for NovaScheduler", "name", novascheduler.GetName()) @@ -93,7 +91,7 @@ var _ webhook.CustomValidator = &NovaSchedulerCustomValidator{} func (v *NovaSchedulerCustomValidator) ValidateCreate(_ context.Context, obj runtime.Object) (admission.Warnings, error) { novascheduler, ok := obj.(*novav1beta1.NovaScheduler) if !ok { - return nil, fmt.Errorf("%w but got %T", errExpectedNovaSchedulerObject, obj) + return nil, fmt.Errorf("expected a NovaScheduler object but got %T: %w", obj, internalcommon.ErrUnexpectedObjectType) } novaschedulerlog.Info("Validation for NovaScheduler upon creation", "name", novascheduler.GetName()) @@ -104,7 +102,7 @@ func (v *NovaSchedulerCustomValidator) ValidateCreate(_ context.Context, obj run func (v *NovaSchedulerCustomValidator) ValidateUpdate(_ context.Context, oldObj, newObj runtime.Object) (admission.Warnings, error) { novascheduler, ok := newObj.(*novav1beta1.NovaScheduler) if !ok { - return nil, fmt.Errorf("%w for the newObj but got %T", errExpectedNovaSchedulerObject, newObj) + return nil, fmt.Errorf("expected a NovaScheduler object for the newObj but got %T: %w", newObj, internalcommon.ErrUnexpectedObjectType) } novaschedulerlog.Info("Validation for NovaScheduler upon update", "name", novascheduler.GetName()) @@ -115,7 +113,7 @@ func (v *NovaSchedulerCustomValidator) ValidateUpdate(_ context.Context, oldObj, func (v *NovaSchedulerCustomValidator) ValidateDelete(_ context.Context, obj runtime.Object) (admission.Warnings, error) { novascheduler, ok := obj.(*novav1beta1.NovaScheduler) if !ok { - return nil, fmt.Errorf("%w but got %T", errExpectedNovaSchedulerObject, obj) + return nil, fmt.Errorf("expected a NovaScheduler object but got %T: %w", obj, internalcommon.ErrUnexpectedObjectType) } novaschedulerlog.Info("Validation for NovaScheduler upon deletion", "name", novascheduler.GetName()) diff --git a/internal/webhook/placement/v1beta1/api_webhook.go b/internal/webhook/placement/v1beta1/api_webhook.go index 314bc3d28..3601fd1e1 100644 --- a/internal/webhook/placement/v1beta1/api_webhook.go +++ b/internal/webhook/placement/v1beta1/api_webhook.go @@ -19,7 +19,6 @@ package v1beta1 import ( "context" - "errors" "fmt" "k8s.io/apimachinery/pkg/runtime" @@ -29,11 +28,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/webhook/admission" placementv1beta1 "github.com/openstack-k8s-operators/nova-operator/api/placement/v1beta1" -) - -var ( - // ErrInvalidObjectType is returned when an unexpected object type is provided - ErrInvalidObjectType = errors.New("invalid object type") + internalcommon "github.com/openstack-k8s-operators/nova-operator/internal/common" ) // nolint:unused @@ -66,7 +61,7 @@ func (d *PlacementAPICustomDefaulter) Default(_ context.Context, obj runtime.Obj placementapi, ok := obj.(*placementv1beta1.PlacementAPI) if !ok { - return fmt.Errorf("expected an PlacementAPI object but got %T: %w", obj, ErrInvalidObjectType) + return fmt.Errorf("expected an PlacementAPI object but got %T: %w", obj, internalcommon.ErrUnexpectedObjectType) } placementapilog.Info("Defaulting for PlacementAPI", "name", placementapi.GetName()) @@ -96,7 +91,7 @@ var _ webhook.CustomValidator = &PlacementAPICustomValidator{} func (v *PlacementAPICustomValidator) ValidateCreate(_ context.Context, obj runtime.Object) (admission.Warnings, error) { placementapi, ok := obj.(*placementv1beta1.PlacementAPI) if !ok { - return nil, fmt.Errorf("expected a PlacementAPI object but got %T: %w", obj, ErrInvalidObjectType) + return nil, fmt.Errorf("expected a PlacementAPI object but got %T: %w", obj, internalcommon.ErrUnexpectedObjectType) } placementapilog.Info("Validation for PlacementAPI upon creation", "name", placementapi.GetName()) @@ -108,7 +103,7 @@ func (v *PlacementAPICustomValidator) ValidateCreate(_ context.Context, obj runt func (v *PlacementAPICustomValidator) ValidateUpdate(_ context.Context, oldObj, newObj runtime.Object) (admission.Warnings, error) { placementapi, ok := newObj.(*placementv1beta1.PlacementAPI) if !ok { - return nil, fmt.Errorf("expected a PlacementAPI object for the newObj but got %T: %w", newObj, ErrInvalidObjectType) + return nil, fmt.Errorf("expected a PlacementAPI object for the newObj but got %T: %w", newObj, internalcommon.ErrUnexpectedObjectType) } placementapilog.Info("Validation for PlacementAPI upon update", "name", placementapi.GetName()) @@ -120,7 +115,7 @@ func (v *PlacementAPICustomValidator) ValidateUpdate(_ context.Context, oldObj, func (v *PlacementAPICustomValidator) ValidateDelete(_ context.Context, obj runtime.Object) (admission.Warnings, error) { placementapi, ok := obj.(*placementv1beta1.PlacementAPI) if !ok { - return nil, fmt.Errorf("expected a PlacementAPI object but got %T: %w", obj, ErrInvalidObjectType) + return nil, fmt.Errorf("expected a PlacementAPI object but got %T: %w", obj, internalcommon.ErrUnexpectedObjectType) } placementapilog.Info("Validation for PlacementAPI upon deletion", "name", placementapi.GetName()) diff --git a/test/functional/placement/api_controller_test.go b/test/functional/placement/api_controller_test.go index 5ff3d0a48..5fd504425 100644 --- a/test/functional/placement/api_controller_test.go +++ b/test/functional/placement/api_controller_test.go @@ -176,6 +176,35 @@ var _ = Describe("PlacementAPI controller", func() { }) }) + When("a PlacementAPI CR is created pointing to a non existent Secret", func() { + BeforeEach(func() { + DeferCleanup( + th.DeleteInstance, + CreatePlacementAPI(names.PlacementAPIName, GetDefaultPlacementAPISpec()), + ) + }) + + It("is not Ready", func() { + th.ExpectCondition( + names.PlacementAPIName, + ConditionGetterFunc(PlacementConditionGetter), + condition.ReadyCondition, + corev1.ConditionFalse, + ) + }) + + It("is missing the secret", func() { + th.ExpectConditionWithDetails( + names.PlacementAPIName, + ConditionGetterFunc(PlacementConditionGetter), + condition.InputReadyCondition, + corev1.ConditionFalse, + condition.ErrorReason, + fmt.Sprintf("Input data resources missing: secret/%s", SecretName), + ) + }) + }) + When("a secret is provided with missing fields", func() { BeforeEach(func() { DeferCleanup( @@ -190,15 +219,20 @@ var _ = Describe("PlacementAPI controller", func() { ) }) It("reports that input is not ready", func() { - // FIXME(gibi): This is a bug as placement controller does not - // check the content of the Secret so eventually a dbsync job is - // created with incorrect config th.ExpectCondition( names.PlacementAPIName, ConditionGetterFunc(PlacementConditionGetter), condition.InputReadyCondition, corev1.ConditionFalse, ) + th.ExpectConditionWithDetails( + names.PlacementAPIName, + ConditionGetterFunc(PlacementConditionGetter), + condition.InputReadyCondition, + corev1.ConditionFalse, + condition.ErrorReason, + fmt.Sprintf("Input data error occurred field not found in Secret: 'PlacementPassword' not found in secret/%s", SecretName), + ) }) }) @@ -1749,11 +1783,11 @@ var _ = Describe("PlacementAPI reconfiguration", func() { mariadb.SimulateMariaDBAccountCompleted(names.MariaDBAccount) }) - It("should set ServiceConfigReady to False", func() { + It("should set InputReady to False while waiting for the ApplicationCredential secret", func() { th.ExpectCondition( names.PlacementAPIName, ConditionGetterFunc(PlacementConditionGetter), - condition.ServiceConfigReadyCondition, + condition.InputReadyCondition, corev1.ConditionFalse, ) }) diff --git a/test/functional/placement/suite_test.go b/test/functional/placement/suite_test.go index 2b933ed9f..1f4088a8e 100644 --- a/test/functional/placement/suite_test.go +++ b/test/functional/placement/suite_test.go @@ -48,6 +48,7 @@ import ( test "github.com/openstack-k8s-operators/lib-common/modules/test" mariadbv1 "github.com/openstack-k8s-operators/mariadb-operator/api/v1beta1" placementv1 "github.com/openstack-k8s-operators/nova-operator/api/placement/v1beta1" + internalcommon "github.com/openstack-k8s-operators/nova-operator/internal/common" controllers "github.com/openstack-k8s-operators/nova-operator/internal/controller/placement" webhookv1 "github.com/openstack-k8s-operators/nova-operator/internal/webhook/placement/v1beta1" @@ -202,9 +203,7 @@ var _ = BeforeSuite(func() { placementv1.SetupDefaults() err = (&controllers.PlacementAPIReconciler{ - Client: k8sManager.GetClient(), - Scheme: k8sManager.GetScheme(), - Kclient: kclient, + ReconcilerBase: internalcommon.NewReconcilerBase(k8sManager, kclient), }).SetupWithManager(k8sManager) Expect(err).ToNot(HaveOccurred()) diff --git a/test/unit/common/helpers_test.go b/test/unit/common/helpers_test.go new file mode 100644 index 000000000..7a6723484 --- /dev/null +++ b/test/unit/common/helpers_test.go @@ -0,0 +1,133 @@ +/* +Copyright 2026. + +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 common_test + +import ( + "context" + "errors" + "testing" + + networkv1 "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1" + topologyv1 "github.com/openstack-k8s-operators/infra-operator/apis/topology/v1beta1" + helper "github.com/openstack-k8s-operators/lib-common/modules/common/helper" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/kubernetes/scheme" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/log" +) + +const testNamespace = "test-ns" + +var ( + errTestSecretGet = errors.New("connection refused") + errTestNADGet = errors.New("apiserver unavailable") +) + +func newTestInstance() *corev1.ConfigMap { + return &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-instance", + Namespace: testNamespace, + }, + } +} + +func newTestHelper(t *testing.T, s *runtime.Scheme, objs ...client.Object) *helper.Helper { + t.Helper() + + instance := newTestInstance() + allObjs := append([]client.Object{instance}, objs...) + cl := fake.NewClientBuilder().WithScheme(s).WithObjects(allObjs...).Build() + + h, err := helper.NewHelper(instance, cl, nil, s, log.Log) + if err != nil { + t.Fatalf("helper.NewHelper: %v", err) + } + return h +} + +func newNetworkTestScheme(t *testing.T) *runtime.Scheme { + t.Helper() + + s := runtime.NewScheme() + if err := scheme.AddToScheme(s); err != nil { + t.Fatalf("add core scheme: %v", err) + } + if err := networkv1.AddToScheme(s); err != nil { + t.Fatalf("add network scheme: %v", err) + } + return s +} + +func newTopologyTestScheme(t *testing.T) *runtime.Scheme { + t.Helper() + + s := runtime.NewScheme() + if err := scheme.AddToScheme(s); err != nil { + t.Fatalf("add core scheme: %v", err) + } + if err := topologyv1.AddToScheme(s); err != nil { + t.Fatalf("add topology scheme: %v", err) + } + return s +} + +// errorReader returns a fixed error from Get for EnsureSecret tests. +type errorReader struct { + err error +} + +func (r errorReader) Get(_ context.Context, _ client.ObjectKey, _ client.Object, _ ...client.GetOption) error { + return r.err +} + +func (r errorReader) List(_ context.Context, _ client.ObjectList, _ ...client.ListOption) error { + return r.err +} + +// errorInjectingClient fails Get for NetworkAttachmentDefinition objects. +type errorInjectingClient struct { + client.Client + getErr error +} + +func (c *errorInjectingClient) Get(ctx context.Context, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error { + if c.getErr != nil { + if _, ok := obj.(*networkv1.NetworkAttachmentDefinition); ok { + return c.getErr + } + } + return c.Client.Get(ctx, key, obj, opts...) +} + +func newTestHelperWithGetError(t *testing.T, s *runtime.Scheme, getErr error, objs ...client.Object) *helper.Helper { + t.Helper() + + instance := newTestInstance() + allObjs := append([]client.Object{instance}, objs...) + base := fake.NewClientBuilder().WithScheme(s).WithObjects(allObjs...).Build() + cl := &errorInjectingClient{Client: base, getErr: getErr} + + h, err := helper.NewHelper(instance, cl, nil, s, log.Log) + if err != nil { + t.Fatalf("helper.NewHelper: %v", err) + } + return h +} diff --git a/test/unit/common/network_test.go b/test/unit/common/network_test.go new file mode 100644 index 000000000..5a724d5a0 --- /dev/null +++ b/test/unit/common/network_test.go @@ -0,0 +1,162 @@ +/* +Copyright 2026. + +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 common_test + +import ( + "context" + "fmt" + "strings" + "testing" + "time" + + networkv1 "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1" + condition "github.com/openstack-k8s-operators/lib-common/modules/common/condition" + internalcommon "github.com/openstack-k8s-operators/nova-operator/internal/common" + corev1 "k8s.io/api/core/v1" + k8s_errors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + ctrl "sigs.k8s.io/controller-runtime" +) + +func TestEnsureNetworkAttachments_noAttachments(t *testing.T) { + ctx := context.Background() + requeueTimeout := 5 * time.Second + conditions := &condition.Conditions{} + h := newTestHelper(t, newNetworkTestScheme(t)) + + annotations, result, err := internalcommon.EnsureNetworkAttachments( + ctx, h, nil, conditions, requeueTimeout, + ) + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + if result != (ctrl.Result{}) { + t.Fatalf("expected empty result, got %#v", result) + } + want := map[string]string{networkv1.NetworkAttachmentAnnot: "[]"} + if annotations == nil || annotations[networkv1.NetworkAttachmentAnnot] != want[networkv1.NetworkAttachmentAnnot] { + t.Fatalf("expected annotations %v, got %v", want, annotations) + } + if conditions.Get(condition.NetworkAttachmentsReadyCondition) != nil { + t.Fatal("expected NetworkAttachmentsReady condition to be unset") + } +} + +func TestEnsureNetworkAttachments_missingNAD(t *testing.T) { + const nadName = "missing-nad" + ctx := context.Background() + requeueTimeout := 5 * time.Second + conditions := &condition.Conditions{} + h := newTestHelper(t, newNetworkTestScheme(t)) + + annotations, result, err := internalcommon.EnsureNetworkAttachments( + ctx, h, []string{nadName}, conditions, requeueTimeout, + ) + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + if result.RequeueAfter != requeueTimeout { + t.Fatalf("expected RequeueAfter %s, got %s", requeueTimeout, result.RequeueAfter) + } + if annotations != nil { + t.Fatalf("expected nil annotations, got %v", annotations) + } + + nadReady := conditions.Get(condition.NetworkAttachmentsReadyCondition) + if nadReady == nil { + t.Fatal("expected NetworkAttachmentsReady condition to be set") + } + if nadReady.Status != corev1.ConditionFalse { + t.Fatalf("expected status False, got %q", nadReady.Status) + } + if nadReady.Reason != condition.ErrorReason { + t.Fatalf("expected reason %q, got %q", condition.ErrorReason, nadReady.Reason) + } + wantMessage := fmt.Sprintf(condition.NetworkAttachmentsReadyWaitingMessage, nadName) + if nadReady.Message != wantMessage { + t.Fatalf("expected message %q, got %q", wantMessage, nadReady.Message) + } +} + +func TestEnsureNetworkAttachments_getError(t *testing.T) { + const nadName = "broken-nad" + ctx := context.Background() + requeueTimeout := 5 * time.Second + conditions := &condition.Conditions{} + getErr := k8s_errors.NewInternalError(errTestNADGet) + h := newTestHelperWithGetError(t, newNetworkTestScheme(t), getErr) + + annotations, result, err := internalcommon.EnsureNetworkAttachments( + ctx, h, []string{nadName}, conditions, requeueTimeout, + ) + if err == nil { + t.Fatal("expected error, got nil") + } + if !strings.Contains(err.Error(), "apiserver unavailable") { + t.Fatalf("expected wrapped apiserver error, got %v", err) + } + if result != (ctrl.Result{}) { + t.Fatalf("expected empty result, got %#v", result) + } + if annotations != nil { + t.Fatalf("expected nil annotations, got %v", annotations) + } + + nadReady := conditions.Get(condition.NetworkAttachmentsReadyCondition) + if nadReady == nil { + t.Fatal("expected NetworkAttachmentsReady condition to be set") + } + wantMessage := fmt.Sprintf(condition.NetworkAttachmentsErrorMessage, err.Error()) + if nadReady.Message != wantMessage { + t.Fatalf("expected condition message %q, got %q", wantMessage, nadReady.Message) + } +} + +func TestEnsureNetworkAttachments_nadPresent(t *testing.T) { + const nadName = "internalapi" + ctx := context.Background() + requeueTimeout := 5 * time.Second + conditions := &condition.Conditions{} + scheme := newNetworkTestScheme(t) + nad := &networkv1.NetworkAttachmentDefinition{ + ObjectMeta: metav1.ObjectMeta{ + Name: nadName, + Namespace: testNamespace, + }, + } + h := newTestHelper(t, scheme, nad) + + annotations, result, err := internalcommon.EnsureNetworkAttachments( + ctx, h, []string{nadName}, conditions, requeueTimeout, + ) + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + if result != (ctrl.Result{}) { + t.Fatalf("expected empty result, got %#v", result) + } + want := fmt.Sprintf( + `[{"name":"%s","namespace":"%s","interface":"%s"}]`, + nadName, testNamespace, nadName, + ) + if annotations == nil || annotations[networkv1.NetworkAttachmentAnnot] != want { + t.Fatalf("expected annotation %q, got %v", want, annotations) + } + if conditions.Get(condition.NetworkAttachmentsReadyCondition) != nil { + t.Fatal("expected NetworkAttachmentsReady condition to be unset") + } +} diff --git a/test/unit/common/secret_test.go b/test/unit/common/secret_test.go new file mode 100644 index 000000000..a59e05116 --- /dev/null +++ b/test/unit/common/secret_test.go @@ -0,0 +1,214 @@ +/* +Copyright 2026. + +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 common_test + +import ( + "context" + "errors" + "fmt" + "testing" + "time" + + condition "github.com/openstack-k8s-operators/lib-common/modules/common/condition" + internalcommon "github.com/openstack-k8s-operators/nova-operator/internal/common" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client/fake" +) + +// placement uses EnsureSecret for the OpenStack password secret referenced in +// PlacementAPI.Spec.Secret with PasswordSelectors.Service defaulting to +// PlacementPassword. +func TestEnsureSecret_placementMissingSecret(t *testing.T) { + const ( + namespace = "test-ns" + secretName = "test-osp-secret" + servicePasswordKey = "PlacementPassword" + ) + ctx := context.Background() + requeueTimeout := 5 * time.Second + conditions := &condition.Conditions{} + reader := fake.NewClientBuilder().Build() + + hash, result, _, err := internalcommon.EnsureSecret( + ctx, + types.NamespacedName{Namespace: namespace, Name: secretName}, + []string{servicePasswordKey}, + reader, + conditions, + requeueTimeout, + ) + + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + if hash != "" { + t.Fatalf("expected empty hash, got %q", hash) + } + if result.RequeueAfter != requeueTimeout { + t.Fatalf("expected RequeueAfter %s, got %s", requeueTimeout, result.RequeueAfter) + } + + inputReady := conditions.Get(condition.InputReadyCondition) + if inputReady == nil { + t.Fatal("expected InputReady condition to be set") + } + if inputReady.Status != corev1.ConditionFalse { + t.Fatalf("expected InputReady status False, got %q", inputReady.Status) + } + if inputReady.Reason != condition.ErrorReason { + t.Fatalf("expected reason %q, got %q", condition.ErrorReason, inputReady.Reason) + } + wantMessage := fmt.Sprintf("Input data resources missing: secret/%s", secretName) + if inputReady.Message != wantMessage { + t.Fatalf("expected message %q, got %q", wantMessage, inputReady.Message) + } +} + +func TestEnsureSecret_getError(t *testing.T) { + const ( + namespace = "test-ns" + secretName = "test-osp-secret" + servicePasswordKey = "PlacementPassword" + ) + ctx := context.Background() + requeueTimeout := 5 * time.Second + conditions := &condition.Conditions{} + getErr := errTestSecretGet + + hash, result, _, err := internalcommon.EnsureSecret( + ctx, + types.NamespacedName{Namespace: namespace, Name: secretName}, + []string{servicePasswordKey}, + errorReader{err: getErr}, + conditions, + requeueTimeout, + ) + + if !errors.Is(err, getErr) { + t.Fatalf("expected error %v, got %v", getErr, err) + } + if hash != "" { + t.Fatalf("expected empty hash, got %q", hash) + } + if result != (ctrl.Result{}) { + t.Fatalf("expected empty result, got %#v", result) + } + + inputReady := conditions.Get(condition.InputReadyCondition) + if inputReady == nil { + t.Fatal("expected InputReady condition to be set") + } + wantMessage := fmt.Sprintf("Input data error occurred %s", getErr.Error()) + if inputReady.Message != wantMessage { + t.Fatalf("expected condition message %q, got %q", wantMessage, inputReady.Message) + } +} + +func TestEnsureSecret_placementMissingField(t *testing.T) { + const ( + namespace = "test-ns" + secretName = "test-osp-secret" + servicePasswordKey = "PlacementPassword" + ) + ctx := context.Background() + requeueTimeout := 5 * time.Second + conditions := &condition.Conditions{} + reader := fake.NewClientBuilder().WithObjects(&corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: secretName, + Namespace: namespace, + }, + Data: map[string][]byte{}, + }).Build() + + hash, result, _, err := internalcommon.EnsureSecret( + ctx, + types.NamespacedName{Namespace: namespace, Name: secretName}, + []string{servicePasswordKey}, + reader, + conditions, + requeueTimeout, + ) + + wantErr := fmt.Sprintf("field not found in Secret: '%s' not found in secret/%s", servicePasswordKey, secretName) + if err == nil { + t.Fatal("expected error, got nil") + } + if err.Error() != wantErr { + t.Fatalf("expected error %q, got %q", wantErr, err.Error()) + } + if hash != "" { + t.Fatalf("expected empty hash, got %q", hash) + } + if result != (ctrl.Result{}) { + t.Fatalf("expected empty result, got %#v", result) + } + + inputReady := conditions.Get(condition.InputReadyCondition) + if inputReady == nil { + t.Fatal("expected InputReady condition to be set") + } + wantMessage := fmt.Sprintf("Input data error occurred %s", wantErr) + if inputReady.Message != wantMessage { + t.Fatalf("expected condition message %q, got %q", wantMessage, inputReady.Message) + } +} + +func TestEnsureSecret_placementSecretPresent(t *testing.T) { + const ( + namespace = "test-ns" + secretName = "test-osp-secret" + servicePasswordKey = "PlacementPassword" + ) + ctx := context.Background() + requeueTimeout := 5 * time.Second + conditions := &condition.Conditions{} + reader := fake.NewClientBuilder().WithObjects(&corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: secretName, + Namespace: namespace, + }, + Data: map[string][]byte{ + servicePasswordKey: []byte("12345678"), + }, + }).Build() + + hash, result, secret, err := internalcommon.EnsureSecret( + ctx, + types.NamespacedName{Namespace: namespace, Name: secretName}, + []string{servicePasswordKey}, + reader, + conditions, + requeueTimeout, + ) + + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + if hash == "" { + t.Fatal("expected non-empty hash") + } + if result != (ctrl.Result{}) { + t.Fatalf("expected empty result, got %#v", result) + } + if string(secret.Data[servicePasswordKey]) != "12345678" { + t.Fatalf("unexpected secret data: %#v", secret.Data) + } +} diff --git a/test/unit/common/topology_test.go b/test/unit/common/topology_test.go new file mode 100644 index 000000000..bb1f76cad --- /dev/null +++ b/test/unit/common/topology_test.go @@ -0,0 +1,157 @@ +/* +Copyright 2026. + +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 common_test + +import ( + "context" + "fmt" + "strings" + "testing" + + topologyv1 "github.com/openstack-k8s-operators/infra-operator/apis/topology/v1beta1" + condition "github.com/openstack-k8s-operators/lib-common/modules/common/condition" + internalcommon "github.com/openstack-k8s-operators/nova-operator/internal/common" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +type fakeTopologyHandler struct { + specRef *topologyv1.TopoRef + lastApplied *topologyv1.TopoRef + lastAppliedSet *topologyv1.TopoRef +} + +func (f *fakeTopologyHandler) GetSpecTopologyRef() *topologyv1.TopoRef { + return f.specRef +} + +func (f *fakeTopologyHandler) GetLastAppliedTopology() *topologyv1.TopoRef { + return f.lastApplied +} + +func (f *fakeTopologyHandler) SetLastAppliedTopology(t *topologyv1.TopoRef) { + f.lastAppliedSet = t +} + +func TestEnsureTopology_noTopologyRef(t *testing.T) { + ctx := context.Background() + conditions := &condition.Conditions{} + h := newTestHelper(t, newTopologyTestScheme(t)) + handler := &fakeTopologyHandler{ + lastApplied: &topologyv1.TopoRef{Name: "old", Namespace: testNamespace}, + } + + topology, err := internalcommon.EnsureTopology( + ctx, h, handler, "placementapi", conditions, metav1.LabelSelector{}, + ) + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + if topology != nil { + t.Fatalf("expected nil topology, got %#v", topology) + } + if handler.lastAppliedSet != nil { + t.Fatalf("expected last applied topology cleared, got %#v", handler.lastAppliedSet) + } + if conditions.Get(condition.TopologyReadyCondition) != nil { + t.Fatal("expected TopologyReady condition to be unset") + } +} + +func TestEnsureTopology_missingTopology(t *testing.T) { + const topologyName = "missing-topology" + ctx := context.Background() + conditions := &condition.Conditions{} + h := newTestHelper(t, newTopologyTestScheme(t)) + handler := &fakeTopologyHandler{ + specRef: &topologyv1.TopoRef{ + Name: topologyName, + Namespace: testNamespace, + }, + } + + topology, err := internalcommon.EnsureTopology( + ctx, h, handler, "placementapi", conditions, metav1.LabelSelector{}, + ) + if err == nil { + t.Fatal("expected error, got nil") + } + if !strings.Contains(err.Error(), "waiting for Topology requirements") { + t.Fatalf("expected wrapped topology wait error, got %v", err) + } + if topology != nil { + t.Fatalf("expected nil topology, got %#v", topology) + } + + topologyReady := conditions.Get(condition.TopologyReadyCondition) + if topologyReady == nil { + t.Fatal("expected TopologyReady condition to be set") + } + if topologyReady.Status != corev1.ConditionFalse { + t.Fatalf("expected status False, got %q", topologyReady.Status) + } + if topologyReady.Reason != condition.ErrorReason { + t.Fatalf("expected reason %q, got %q", condition.ErrorReason, topologyReady.Reason) + } + wantPrefix := fmt.Sprintf(condition.TopologyReadyErrorMessage, "") + if !strings.HasPrefix(topologyReady.Message, strings.TrimSuffix(wantPrefix, "%s")) { + t.Fatalf("unexpected condition message %q", topologyReady.Message) + } +} + +func TestEnsureTopology_topologyPresent(t *testing.T) { + const topologyName = "az-one" + ctx := context.Background() + conditions := &condition.Conditions{} + scheme := newTopologyTestScheme(t) + topologyCR := &topologyv1.Topology{ + ObjectMeta: metav1.ObjectMeta{ + Name: topologyName, + Namespace: testNamespace, + }, + } + h := newTestHelper(t, scheme, topologyCR) + specRef := &topologyv1.TopoRef{ + Name: topologyName, + Namespace: testNamespace, + } + handler := &fakeTopologyHandler{specRef: specRef} + + topology, err := internalcommon.EnsureTopology( + ctx, h, handler, "placementapi", conditions, metav1.LabelSelector{}, + ) + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + if topology == nil || topology.Name != topologyName { + t.Fatalf("expected topology %q, got %#v", topologyName, topology) + } + if handler.lastAppliedSet == nil || handler.lastAppliedSet.Name != topologyName { + t.Fatalf("expected last applied topology %q, got %#v", topologyName, handler.lastAppliedSet) + } + + topologyReady := conditions.Get(condition.TopologyReadyCondition) + if topologyReady == nil { + t.Fatal("expected TopologyReady condition to be set") + } + if topologyReady.Status != corev1.ConditionTrue { + t.Fatalf("expected status True, got %q", topologyReady.Status) + } + if topologyReady.Message != condition.TopologyReadyMessage { + t.Fatalf("expected message %q, got %q", condition.TopologyReadyMessage, topologyReady.Message) + } +}