MON-1261: exposing topology spread constraints through config #1624

jan--f · 2022-03-31T11:17:06Z

This PR exposes the TopologySpreadConstraints through the CMO config.

This is just #1469 rebased on master.

I added CHANGELOG entry for this change.
No user facing changes, so no entry in CHANGELOG was needed.

raptorsun · 2022-04-01T12:57:08Z

/lgtm

openshift-bot · 2022-04-01T15:03:08Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-04-01T17:39:06Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-04-01T19:36:05Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-04-01T20:41:05Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-04-02T00:47:40Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-04-02T02:47:41Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-04-02T03:11:44Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-04-02T04:35:42Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-04-02T04:47:40Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-04-02T06:16:39Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-04-02T08:00:45Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-04-02T08:13:43Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-04-02T09:31:41Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-04-02T11:28:40Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-04-02T11:41:41Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-04-02T13:12:41Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-04-02T13:25:42Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-04-02T14:37:40Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-04-02T15:25:40Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-04-02T17:13:41Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-04-02T18:49:42Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-04-02T19:37:41Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-04-02T19:49:41Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-04-02T21:49:41Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-04-04T05:33:42Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-04-04T05:45:44Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-04-04T06:09:41Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

jan--f · 2022-04-04T07:19:14Z

/hold
for approved labels

openshift-ci · 2022-04-23T21:24:47Z

@jan--f: PR needs rebase.

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/test-infra repository.

sthaha · 2022-05-03T02:30:04Z

pkg/manifests/manifests.go

@@ -534,6 +534,11 @@ func (f *Factory) AlertmanagerMain(trustedCABundleCM *v1.ConfigMap) (*monv1.Aler
 		a.Spec.Tolerations = f.config.ClusterMonitoringConfiguration.AlertmanagerMainConfig.Tolerations
 	}

+	if len(f.config.ClusterMonitoringConfiguration.AlertmanagerMainConfig.TopologySpreadConstraints) > 0 {


should we check the len ? Wouldn't a.Spec.TopologySpreadConstraints = f.config.ClusterMonitoringConfiguration.AlertmanagerMainConfig.TopologySpreadConstraints yield the same result?

We do check len in other places in CMO too. Iiuc what this does is, if an empty slice is passed from the user, it gets normalized to nil. Maybe that provides a small optimization for the resulting json? The nil value can be omitted instead of having to encode and empty slice?
Not sure, but I'd opt to stay consistent with the other places here.

yes the len(...) > 0 check is to handle nil and empty slices altogether (e.g. topologySpreadConstraints: [] and topologySpreadConstraints: nil).

sthaha

/lgtm

A nit that you may want to consider addressing.

juzhao · 2022-05-13T09:59:34Z

/label qe-approved

faust64 · 2022-07-20T08:25:30Z

Any news?
This would be quite useful.
When I last upgraded OpenShift clusters to 4.10, I got a few clusters stuck running pre-upgrade checks. Upgrade would not start, if all pods from an alertmanager, prometheus or thanos statefulset are located on the same node.
Which I could not believe: there isn't even a kubernetes.io/hostname anti-affinity rule? Nor any way, yet, to configure affinities for cluster-monitoring components?

jan--f · 2022-07-26T06:17:25Z

@faust64 please follow https://issues.redhat.com/browse/MON-1261. I'll update the code and get it merged soon. However this will only make it into the 4.12 release due to documentation and QE work.

This commit also adds unit test cases and support in thanos ruler.

jan--f · 2022-07-26T12:21:35Z

/retest

openshift-ci · 2022-07-26T14:48:20Z

@jan--f: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/e2e-aws-single-node	`9497272`	link	false	`/test e2e-aws-single-node`

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/test-infra repository. I understand the commands that are listed here.

sthaha · 2022-07-27T05:58:19Z

/lgtm

openshift-ci · 2022-07-27T05:59:02Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jan--f, raptorsun, sthaha

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [jan--f,raptorsun,sthaha]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

jan--f · 2022-08-04T13:01:12Z

/unhold
/label docs-approved
Adding doc approval as per agreement.
Ping @juzhao for qe approval.
Ping @Senthamilarasu-STA for px approval.
https://issues.redhat.com/browse/MON-1261

Senthamilarasu-STA · 2022-08-04T13:59:17Z

/label px-approved

openshift-ci bot requested review from simonpasquier and slashpai March 31, 2022 11:17

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 31, 2022

openshift-ci bot assigned raptorsun Apr 1, 2022

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Apr 1, 2022

openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 4, 2022

openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 23, 2022

sthaha reviewed May 3, 2022

View reviewed changes

openshift-ci bot assigned sthaha May 3, 2022

openshift-ci bot added the qe-approved Signifies that QE has signed off on this PR label May 13, 2022

jan--f force-pushed the mon-1261 branch from 06e6fd8 to 94f6a48 Compare July 26, 2022 06:42

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Jul 26, 2022

openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 26, 2022

MON-1261: expose topology spread constraints through config

9497272

This commit also adds unit test cases and support in thanos ruler.

jan--f force-pushed the mon-1261 branch from 94f6a48 to 9497272 Compare July 26, 2022 06:49

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jul 27, 2022

openshift-ci bot added docs-approved Signifies that Docs has signed off on this PR and removed do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. labels Aug 4, 2022

openshift-ci bot added the px-approved Signifies that Product Support has signed off on this PR label Aug 4, 2022

openshift-ci bot merged commit f43c87e into openshift:master Aug 4, 2022

MON-1261: exposing topology spread constraints through config #1624

MON-1261: exposing topology spread constraints through config #1624

Conversation

jan--f commented Mar 31, 2022

raptorsun commented Apr 1, 2022

openshift-bot commented Apr 1, 2022

openshift-bot commented Apr 1, 2022

openshift-bot commented Apr 1, 2022

openshift-bot commented Apr 1, 2022

openshift-bot commented Apr 2, 2022

openshift-bot commented Apr 2, 2022

openshift-bot commented Apr 2, 2022

openshift-bot commented Apr 2, 2022

openshift-bot commented Apr 2, 2022

openshift-bot commented Apr 2, 2022

openshift-bot commented Apr 2, 2022

openshift-bot commented Apr 2, 2022

openshift-bot commented Apr 2, 2022

openshift-bot commented Apr 2, 2022

openshift-bot commented Apr 2, 2022

openshift-bot commented Apr 2, 2022

openshift-bot commented Apr 2, 2022

openshift-bot commented Apr 2, 2022

openshift-bot commented Apr 2, 2022

openshift-bot commented Apr 2, 2022

openshift-bot commented Apr 2, 2022

openshift-bot commented Apr 2, 2022

openshift-bot commented Apr 2, 2022

openshift-bot commented Apr 2, 2022

openshift-bot commented Apr 4, 2022

openshift-bot commented Apr 4, 2022

openshift-bot commented Apr 4, 2022

jan--f commented Apr 4, 2022

openshift-ci bot commented Apr 23, 2022

sthaha May 3, 2022

Choose a reason for hiding this comment

jan--f Jul 26, 2022

Choose a reason for hiding this comment

simonpasquier Jul 26, 2022

Choose a reason for hiding this comment

sthaha left a comment

Choose a reason for hiding this comment

juzhao commented May 13, 2022

faust64 commented Jul 20, 2022

jan--f commented Jul 26, 2022

jan--f commented Jul 26, 2022

openshift-ci bot commented Jul 26, 2022

sthaha commented Jul 27, 2022

openshift-ci bot commented Jul 27, 2022

jan--f commented Aug 4, 2022

Senthamilarasu-STA commented Aug 4, 2022