Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consume Topology CR #670

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions api/bases/glance.openstack.org_glanceapis.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -704,6 +704,13 @@ spec:
caBundleSecretName:
type: string
type: object
topologyRef:
properties:
name:
type: string
namespace:
type: string
type: object
type:
default: split
enum:
Expand Down
14 changes: 14 additions & 0 deletions api/bases/glance.openstack.org_glances.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -696,6 +696,13 @@ spec:
caBundleSecretName:
type: string
type: object
topologyRef:
properties:
name:
type: string
namespace:
type: string
type: object
type:
default: split
enum:
Expand Down Expand Up @@ -777,6 +784,13 @@ spec:
storageRequest:
type: string
type: object
topologyRef:
properties:
name:
type: string
namespace:
type: string
type: object
required:
- containerImage
- databaseInstance
Expand Down
4 changes: 4 additions & 0 deletions api/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -70,3 +70,7 @@ require (
// mschuppert: map to latest commit from release-4.16 tag
// must consistent within modules and service operators
replace github.com/openshift/api => github.com/openshift/api v0.0.0-20240830023148-b7d0481c9094 //allow-merging

replace github.com/openstack-k8s-operators/infra-operator/apis => github.com/fmount/infra-operator/apis v0.0.0-20241217210734-91376cf9eb3f //allow-merging

replace github.com/openstack-k8s-operators/lib-common/modules/common => github.com/fmount/lib-common/modules/common v0.0.0-20241217100632-a2c8ea43c395 //allow-merging
4 changes: 2 additions & 2 deletions api/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ github.com/evanphx/json-patch v5.7.0+incompatible h1:vgGkfT/9f8zE6tvSCe74nfpAVDQ
github.com/evanphx/json-patch v5.7.0+incompatible/go.mod h1:50XU6AFN0ol/bzJsmQLiYLvXMP4fmwYFNcr97nuDLSk=
github.com/evanphx/json-patch/v5 v5.9.0 h1:kcBlZQbplgElYIlo/n1hJbls2z/1awpXxpRi0/FOJfg=
github.com/evanphx/json-patch/v5 v5.9.0/go.mod h1:VNkHZ/282BpEyt/tObQO8s5CMPmYYq14uClGH4abBuQ=
github.com/fmount/lib-common/modules/common v0.0.0-20241217100632-a2c8ea43c395 h1:FTnFgkzbg5agJorJB4wXfU4LtW8xfGAsozo8It1i6vU=
github.com/fmount/lib-common/modules/common v0.0.0-20241217100632-a2c8ea43c395/go.mod h1:YpNTuJhDWhbXM50O3qBkhO7M+OOyRmWkNVmJ4y3cyFs=
github.com/fsnotify/fsnotify v1.7.0 h1:8JEhPFa5W2WU7YfeZzPNqzMP6Lwt7L2715Ggo0nosvA=
github.com/fsnotify/fsnotify v1.7.0/go.mod h1:40Bi/Hjc2AVfZrqy+aj+yEI+/bRxZnMJyTJwOpGvigM=
github.com/go-logr/logr v1.4.2 h1:6pFjapn8bFcIbiKo3XT4j/BhANplGihG6tvd+8rYgrY=
Expand Down Expand Up @@ -75,8 +77,6 @@ github.com/onsi/gomega v1.34.1 h1:EUMJIKUjM8sKjYbtxQI9A4z2o+rruxnzNvpknOXie6k=
github.com/onsi/gomega v1.34.1/go.mod h1:kU1QgUvBDLXBJq618Xvm2LUX6rSAfRaFRTcdOeDLwwY=
github.com/openshift/api v0.0.0-20240830023148-b7d0481c9094 h1:J1wuGhVxpsHykZBa6Beb1gQ96Ptej9AE/BvwCBiRj1E=
github.com/openshift/api v0.0.0-20240830023148-b7d0481c9094/go.mod h1:CxgbWAlvu2iQB0UmKTtRu1YfepRg1/vJ64n2DlIEVz4=
github.com/openstack-k8s-operators/lib-common/modules/common v0.5.1-0.20241216113837-d172b3ac0f4e h1:hf4kVQBkyG79WcHBxdQ25QrDBbGFdarebS1Tc0Xclq4=
github.com/openstack-k8s-operators/lib-common/modules/common v0.5.1-0.20241216113837-d172b3ac0f4e/go.mod h1:YpNTuJhDWhbXM50O3qBkhO7M+OOyRmWkNVmJ4y3cyFs=
github.com/openstack-k8s-operators/lib-common/modules/storage v0.5.1-0.20241216113837-d172b3ac0f4e h1:Qz0JFEoRDUyjEWorNY3LggwxTsmpMtQkcpmZDQulGHQ=
github.com/openstack-k8s-operators/lib-common/modules/storage v0.5.1-0.20241216113837-d172b3ac0f4e/go.mod h1:tfgBeLRqmlH/NQkLPe7396rj+t0whv2wPuMb8Ttvh8w=
github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4=
Expand Down
20 changes: 20 additions & 0 deletions api/v1beta1/common_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,11 @@ type GlanceAPITemplate struct {
// NodeSelector to target subset of worker nodes running this service
NodeSelector *map[string]string `json:"nodeSelector,omitempty"`

// +kubebuilder:validation:Optional
// Topology to apply the Policy defined by the associated CR referenced by
// name
Topology *TopologyRef `json:"topologyRef,omitempty"`

// +kubebuilder:validation:Optional
// CustomServiceConfig - customize the service config using this parameter to change service defaults,
// or overwrite rendered information using raw OpenStack config format. The content gets added to
Expand Down Expand Up @@ -108,6 +113,21 @@ type GlanceAPITemplate struct {
APITimeout int `json:"apiTimeout,omitempty"`
}

// TopologyRef -
type TopologyRef struct {
// +kubebuilder:validation:Optional
// Name - The Topology CR name that Glance references
Name string `json:"name"`

// +kubebuilder:validation:Optional
// Namespace - The Namespace to fetch the Topology CR referenced
// NOTE: Namespace currently points by default to the same namespace where
// Glance is deployed. Customizing the namespace is not supported and
// webhooks prevent editing this field to a value different from the
// current project
Namespace string `json:"namespace,omitempty"`
fmount marked this conversation as resolved.
Show resolved Hide resolved
}

// Storage -
type Storage struct {
// +kubebuilder:validation:Optional
Expand Down
9 changes: 9 additions & 0 deletions api/v1beta1/conditions.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,4 +43,13 @@ const (
InvalidBackendErrorMessageSplit = "The GlanceAPI layout type: split cannot be used in combination with File and NFS backend"
// InvalidBackendErrorMessageSingle
InvalidBackendErrorMessageSingle = "The GlanceAPI layout type: single can only be used in combination with File and NFS backend"

// TopologyConfigReadyInitMessage
TopologyConfigReadyInitMessage = "Topology config create not started"
// TopologyConfigReadyMessage
TopologyConfigReadyMessage = "Topology config create completed"
// TopologyConfigReadyErrorMessage
TopologyConfigReadyErrorMessage = "Topology config create error occurred %s"
// TopologyConfigReadyCondition Status=True condition which indicates a CR exists and is referenced by the Glance
TopologyConfigReadyCondition condition.Type = "TopologyConfigReady"
)
5 changes: 5 additions & 0 deletions api/v1beta1/glance_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,11 @@ type GlanceSpecCore struct {
// NodeSelector to target subset of worker nodes running this service
NodeSelector *map[string]string `json:"nodeSelector,omitempty"`

// +kubebuilder:validation:Optional
// Topology to apply the Policy defined by the associated CR referenced by
// name
Topology *TopologyRef `json:"topologyRef,omitempty"`

// +kubebuilder:validation:Optional
// +kubebuilder:default=false
// PreserveJobs - do not delete jobs after they finished e.g. to check logs
Expand Down
28 changes: 28 additions & 0 deletions api/v1beta1/glance_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,15 @@ func (r *Glance) ValidateCreate() (admission.Warnings, error) {
var allErrs field.ErrorList
basePath := field.NewPath("spec")

// When a Topology CR is referenced, fail if a different Namespace is
// referenced because is not supported
if err := ValidateTopologyNamespace(r.Spec.Topology, *basePath, r.Namespace); err != nil {
allErrs = append(allErrs, err)
}
for key, glanceAPI := range r.Spec.GlanceAPIs {
if err := ValidateTopologyNamespace(glanceAPI.Topology, *basePath.Child("glanceAPIs"), r.Namespace); err != nil {
allErrs = append(allErrs, err)
}
// Validate glanceapi name is valid
// GlanceAPI name is <glance name>-<api name>-<api type>
// The glanceAPI controller creates StatefulSet for glanceapi to run.
Expand Down Expand Up @@ -307,7 +315,15 @@ func (r *Glance) ValidateUpdate(old runtime.Object) (admission.Warnings, error)
var allErrs field.ErrorList
basePath := field.NewPath("spec")

// When a Topology CR is referenced, fail if a different Namespace is
// referenced because is not supported
if err := ValidateTopologyNamespace(r.Spec.Topology, *basePath, r.Namespace); err != nil {
allErrs = append(allErrs, err)
}
for key, glanceAPI := range r.Spec.GlanceAPIs {
if err := ValidateTopologyNamespace(glanceAPI.Topology, *basePath.Child("glanceAPIs"), r.Namespace); err != nil {
allErrs = append(allErrs, err)
}
// Validate glanceapi name is valid
// GlanceAPI name is <glance name>-<api name>-<api type>
// The glanceAPI controller creates StatefulSet for glanceapi to run.
Expand Down Expand Up @@ -441,3 +457,15 @@ func GetCrMaxLengthCorrection(name string, apiType string) int {

return (defaultCrMaxLengthCorrection + len(name) + len(apiType) + 2)
}

// ValidateTopologyNamespace - returns a field.Error when Glance / GlanceAPI
// references a Topoology deployed on a different namespace
func ValidateTopologyNamespace(topology *TopologyRef, basePath field.Path, ns string) (*field.Error) {
if topology != nil {
if topology.Namespace != "" && topology.Namespace != ns {
topologyNamespace := basePath.Child("topology").Key("namespace")
return field.Invalid(topologyNamespace, "namespace", "Customizing namespace field is not supported")
}
}
return nil
}
25 changes: 25 additions & 0 deletions api/v1beta1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 7 additions & 0 deletions config/crd/bases/glance.openstack.org_glanceapis.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -704,6 +704,13 @@ spec:
caBundleSecretName:
type: string
type: object
topologyRef:
properties:
name:
type: string
namespace:
type: string
type: object
type:
default: split
enum:
Expand Down
14 changes: 14 additions & 0 deletions config/crd/bases/glance.openstack.org_glances.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -696,6 +696,13 @@ spec:
caBundleSecretName:
type: string
type: object
topologyRef:
properties:
name:
type: string
namespace:
type: string
type: object
type:
default: split
enum:
Expand Down Expand Up @@ -777,6 +784,13 @@ spec:
storageRequest:
type: string
type: object
topologyRef:
properties:
name:
type: string
namespace:
type: string
type: object
required:
- containerImage
- databaseInstance
Expand Down
4 changes: 2 additions & 2 deletions config/manager/kustomization.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,5 @@ apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
images:
- name: controller
newName: quay.io/openstack-k8s-operators/glance-operator
newTag: latest
newName: quay.io/fpantano/glance-operator
newTag: v01614fb5fea9
10 changes: 10 additions & 0 deletions config/rbac/role.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -286,3 +286,13 @@ rules:
- securitycontextconstraints
verbs:
- use
- apiGroups:
- topology.openstack.org
resources:
- topologies
verbs:
- get
- list
- patch
- update
- watch
7 changes: 7 additions & 0 deletions controllers/glance_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -866,6 +866,13 @@ func (r *GlanceReconciler) apiDeploymentCreateOrUpdate(
if instance.Spec.KeystoneEndpoint == apiName {
apiAnnotations[glance.KeystoneEndpoint] = "true"
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can the topology of a deployment be updated after it get created, or is it immutable? We should probably watch the topology, like we do with the secrets. if it can be updated, all good, if not we have to think about what we should do in that case.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not immutable and I'm able to see reconciliation loops being triggered by a change in the topology.
Adding the same watching logic here would simply be redundant, because glanceapi_controller (where I pass the topology reference) has already such watcher, and it triggers a reconciliation if something changes even when the topologyRef is a glance top level parameter (tested locally).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah right, have not looked closely at the api controller

// If topology is not present in the underlying GlanceAPI,
// inherit from the top-level CR
if apiSpec.GlanceAPITemplate.Topology == nil {
apiSpec.GlanceAPITemplate.Topology = instance.Spec.Topology
}

// Add the API name to the GlanceAPI instance as a label
serviceLabels[glancev1.APINameLabel] = apiName
glanceStatefulset := &glancev1.GlanceAPI{
Expand Down
Loading
Loading