Skip to content

Conversation

@gkurz
Copy link
Member

@gkurz gkurz commented Sep 19, 2025

This is refactoring peer pods code to a separate file for easier maintenance.

Without the PR :

$ wc -l controllers/openshift_controller.go
2764 controllers/openshift_controller.go

With the PR :

$ wc -l controllers/openshift_controller.go
2220 controllers/openshift_controller.go

No change in behavior.

$ wc -l controllers/openshift_controller.go
2764 controllers/openshift_controller.go

The file is huge already.

The peer-pods logic needs some heavy fixing that will require to add some
more peer-pods specific code.

Let's move peer-pods to a separate file. It will be easier to read and
maintain. This is consistent with what we already do for other more recent
features.

This doesn't change behavior.

Signed-off-by: Greg Kurz <[email protected]>
Consolidate the peer pods enablement code in a separate function.

This doesn't change behavior.

Signed-off-by: Greg Kurz <[email protected]>
Consolidate the peer pods disablement code in a separate function.

For the sake of symmetry with enablePeerPods, name it disablePeerPods. The
existing disablePeerPods is arbitrary renamed to disablePeerPodsMiscConfigs.

Signed-off-by: Greg Kurz <[email protected]>
@openshift-ci openshift-ci bot requested review from c3d and vvoronko September 19, 2025 15:03
@openshift-ci
Copy link

openshift-ci bot commented Sep 19, 2025

@gkurz: all tests passed!

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

@gkurz
Copy link
Member Author

gkurz commented Sep 19, 2025

/hold

so that people can review.

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 19, 2025
Copy link
Member

@beraldoleal beraldoleal left a comment

Choose a reason for hiding this comment

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

super welcome this change, LTGM. thanks.

Copy link
Contributor

@bpradipt bpradipt 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 @gkurz

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Sep 23, 2025
Copy link
Contributor

@littlejawa littlejawa 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 @gkurz

@gkurz gkurz merged commit ad1f438 into openshift:devel Sep 26, 2025
7 checks passed
@gkurz gkurz removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 9, 2025
@gkurz gkurz deleted the refactor-peerpods branch October 28, 2025 16:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants