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

Add TopoRef struct and common functions #594

Merged
merged 1 commit into from
Jan 28, 2025

Conversation

fmount
Copy link
Contributor

@fmount fmount commented Jan 21, 2025

TopologyRef represents a struct referenced by the service operators to retrieve a Topology CR that is eventually processed and applied to the resulting Deployments/StatefulSets.
This patch allows to reduce the amount of code duplication across the operators, providing both TopologyRef struct that can be added in the Services' API, as well as EnsureTopologyRef and EnsureDeletedTopologyRef where the basic logic of retrieving and managing finalizers is implemented.

@fmount fmount requested review from olliewalsh and stuggi January 21, 2025 10:25
@fmount fmount force-pushed the topologyref branch 2 times, most recently from 00e666b to 27cca91 Compare January 21, 2025 10:26
modules/common/topology/types.go Outdated Show resolved Hide resolved
modules/common/topology/topology.go Show resolved Hide resolved
modules/common/topology/topology.go Show resolved Hide resolved
modules/common/topology/types.go Show resolved Hide resolved
@fmount fmount force-pushed the topologyref branch 2 times, most recently from 5618886 to f3ec112 Compare January 21, 2025 11:01
@fmount fmount requested a review from fultonj January 21, 2025 12:37
fmount added a commit to fmount/glance-operator that referenced this pull request Jan 21, 2025
…mmon

This patch updates the glance-operator to consume a TopologyRef struct
and the related functions from lib-common [1]. This allows to avoid
code duplication while spreading the pattern across the board.

[1] openstack-k8s-operators/lib-common#594

Signed-off-by: Francesco Pantano <[email protected]>
modules/common/topology/topology.go Outdated Show resolved Hide resolved
modules/common/topology/topology.go Outdated Show resolved Hide resolved
@fmount fmount force-pushed the topologyref branch 2 times, most recently from 8cbe0a8 to 2005db7 Compare January 21, 2025 15:41
olliewalsh pushed a commit to olliewalsh/glance-operator that referenced this pull request Jan 22, 2025
…mmon

This patch updates the glance-operator to consume a TopologyRef struct
and the related functions from lib-common [1]. This allows to avoid
code duplication while spreading the pattern across the board.

[1] openstack-k8s-operators/lib-common#594

Signed-off-by: Francesco Pantano <[email protected]>
fmount added a commit to fmount/glance-operator that referenced this pull request Jan 22, 2025
…mmon

This patch updates the glance-operator to consume a TopologyRef struct
and the related functions from lib-common [1]. This allows to avoid
code duplication while spreading the pattern across the board.

[1] openstack-k8s-operators/lib-common#594

Signed-off-by: Francesco Pantano <[email protected]>
fmount added a commit to fmount/glance-operator that referenced this pull request Jan 22, 2025
…mmon

This patch updates the glance-operator to consume a TopologyRef struct
and the related functions from lib-common [1]. This allows to avoid
code duplication while spreading the pattern across the board.

[1] openstack-k8s-operators/lib-common#594

Signed-off-by: Francesco Pantano <[email protected]>
fmount added a commit to fmount/glance-operator that referenced this pull request Jan 23, 2025
…mmon

This patch updates the glance-operator to consume a TopologyRef struct
and the related functions from lib-common [1]. This allows to avoid
code duplication while spreading the pattern across the board.

[1] openstack-k8s-operators/lib-common#594

Signed-off-by: Francesco Pantano <[email protected]>
fmount added a commit to fmount/glance-operator that referenced this pull request Jan 23, 2025
…mmon

This patch updates the glance-operator to consume a TopologyRef struct
and the related functions from lib-common [1]. This allows to avoid
code duplication while spreading the pattern across the board.

[1] openstack-k8s-operators/lib-common#594

Signed-off-by: Francesco Pantano <[email protected]>
modules/common/topology/types.go Outdated Show resolved Hide resolved
modules/common/topology/topology.go Show resolved Hide resolved
fmount added a commit to fmount/glance-operator that referenced this pull request Jan 24, 2025
…mmon

This patch updates the glance-operator to consume a TopologyRef struct
and the related functions from lib-common [1]. This allows to avoid
code duplication while spreading the pattern across the board.

[1] openstack-k8s-operators/lib-common#594

Signed-off-by: Francesco Pantano <[email protected]>
@fmount fmount changed the title Add TopologyRef struct and common functions Add TopoRef struct and common functions Jan 24, 2025
@fmount fmount requested review from stuggi and ASBishop January 24, 2025 09:05
fmount added a commit to fmount/glance-operator that referenced this pull request Jan 24, 2025
…mmon

This patch updates the glance-operator to consume a TopologyRef struct
and the related functions from lib-common [1]. This allows to avoid
code duplication while spreading the pattern across the board.

[1] openstack-k8s-operators/lib-common#594

Signed-off-by: Francesco Pantano <[email protected]>
Copy link
Contributor

@stuggi stuggi left a comment

Choose a reason for hiding this comment

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

/lgtm

@fmount fmount force-pushed the topologyref branch 3 times, most recently from 297cfdc to 13607ec Compare January 27, 2025 15:41
TopoRef represents a struct referenced by the service operators to
retrieve a Topology that is eventually applied to the resulting
Deployments/StatefulSets.
This patch allows to reduce the amount of code duplication across the
operators, providing both the TopoRef struct that can be added in the
Services' API, as well as "EnsureTopologyRef" and "EnsureDeletedTopologyRef"
where the basic logic of retrieving and managing finalizers is implemented.

Signed-off-by: Francesco Pantano <[email protected]>
@fmount
Copy link
Contributor Author

fmount commented Jan 27, 2025

@olliewalsh @stuggi this change is ready, and as you can see I cleaned up the implementation for glance that consumes this patch [1].
I understand we might need to follow up here, but considering the timeline we have and the remaining time to spread the pattern across the board I'm wondering if we can land this patch.

[1] openstack-k8s-operators/glance-operator#670

Copy link
Contributor

@ASBishop ASBishop left a comment

Choose a reason for hiding this comment

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

lgtm, thanks!


// no Topology is passed at all or some data is missing
if topologyRef == nil || (topologyRef.Name == "" || topologyRef.Namespace == "") {
return ctrl.Result{}, nil
Copy link
Contributor

Choose a reason for hiding this comment

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

This is treated as an error on L43, but I agree it's OK to "quietly succeed" here and not return an error.

fmount added a commit to fmount/glance-operator that referenced this pull request Jan 28, 2025
…mmon

This patch updates the glance-operator to consume a TopologyRef struct
and the related functions from lib-common [1]. This allows to avoid
code duplication while spreading the pattern across the board.

[1] openstack-k8s-operators/lib-common#594

Signed-off-by: Francesco Pantano <[email protected]>
Copy link
Contributor

@stuggi stuggi left a comment

Choose a reason for hiding this comment

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

/lgtm

@stuggi stuggi merged commit 53b65fc into openstack-k8s-operators:main Jan 28, 2025
2 checks passed
fmount added a commit to fmount/glance-operator that referenced this pull request Jan 28, 2025
…mmon

This patch updates the glance-operator to consume a TopologyRef struct
and the related functions from lib-common [1]. This allows to avoid
code duplication while spreading the pattern across the board.

[1] openstack-k8s-operators/lib-common#594

Signed-off-by: Francesco Pantano <[email protected]>
fmount added a commit to fmount/glance-operator that referenced this pull request Jan 28, 2025
…mmon

This patch updates the glance-operator to consume a TopologyRef struct
and the related functions from lib-common [1]. This allows to avoid
code duplication while spreading the pattern across the board.

[1] openstack-k8s-operators/lib-common#594

Signed-off-by: Francesco Pantano <[email protected]>
fmount added a commit to fmount/glance-operator that referenced this pull request Jan 29, 2025
…mmon

This patch updates the glance-operator to consume a TopologyRef struct
and the related functions from lib-common [1]. This allows to avoid
code duplication while spreading the pattern across the board.

[1] openstack-k8s-operators/lib-common#594

Signed-off-by: Francesco Pantano <[email protected]>
fmount added a commit to fmount/glance-operator that referenced this pull request Jan 29, 2025
…mmon

This patch updates the glance-operator to consume a TopologyRef struct
and the related functions from lib-common [1]. This allows to avoid
code duplication while spreading the pattern across the board.

[1] openstack-k8s-operators/lib-common#594

Signed-off-by: Francesco Pantano <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants