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

Update CC goal documentation #9678

Merged
merged 9 commits into from Feb 14, 2024
Merged

Update CC goal documentation #9678

merged 9 commits into from Feb 14, 2024

Conversation

kyguy
Copy link
Member

@kyguy kyguy commented Feb 12, 2024

Type of change

  • Documentation

Description

Following up the discussion from here [1] and here [2] we have a misunderstanding of how the hard.goals configuration works in Cruise Control. We cannot configure whether a goal is a "soft goal" or a "hard goal". The hard.goals configuration only allows us to specify which goals will be enforced to run by Cruise Control.

For example, if NetworkInboundCapacityGoal is not specified in default.goals list but specified in the hard.goals list, Cruise Control will throw an error. In order to ensure NetworkInboundCapacityGoal is not run, it must be removed from default.goals and hard.goals lists. Cruise Control hard codes which goals are "hard" and which are "soft" (e.g. NetworkInboundCapacityGoal is hard coded as a "hard goal", we cannot configure it as a "soft goal"). Therefore, if a hard goal is listed in the default.goals list, it will prevent a optimization proposal from being generated if it is not satisfied.

This PR updates the documentation to reflect this information.

[1] https://github.com/orgs/strimzi/discussions/9546
[2] linkedin/cruise-control#2111

Checklist

Please go through this checklist and make sure all applicable tasks have been done

  • Write tests
  • Make sure all tests pass
  • Update documentation
  • Check RBAC rights for Kubernetes / OpenShift roles
  • Try your changes from Pod inside your Kubernetes and OpenShift cluster, not just locally
  • Reference relevant issue(s) and close them after merging
  • Update CHANGELOG.md
  • Supply screenshots for visual changes, such as Grafana dashboards

Signed-off-by: Kyle Liberti <kliberti@redhat.com>
@kyguy kyguy added this to the 0.40.0 milestone Feb 12, 2024
Copy link
Member

@scholzj scholzj left a comment

Choose a reason for hiding this comment

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

I would avoid using the hard-coded term here. It is confusing as it mixes with hard goals. It might not also be clear what it means to all users. You can probably say that they are defined as hard goals in Cruise Control code.

@@ -61,7 +61,7 @@ Resource distribution goals are subject to link:{BookURLConfiguring}#property-cr
== Hard and soft optimization goals

Hard goals are goals that _must_ be satisfied in optimization proposals.
Goals that are not configured as hard goals are known as _soft goals_.
Goals that are not hard coded as _hard goals_ in Cruise Control are known as _soft goals_.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Goals that are not hard coded as _hard goals_ in Cruise Control are known as _soft goals_.
Goals that are not defined as _hard goals_ in Cruise Control code are known as _soft goals_.

@@ -71,16 +71,21 @@ An optimization proposal that does _not_ satisfy all the hard goals is rejected
NOTE: For example, you might have a soft goal to distribute a topic's replicas evenly across the cluster (the topic replica distribution goal).
Cruise Control will ignore this goal if doing so enables all the configured hard goals to be met.

In Cruise Control, the following xref:main-goals[main optimization goals] are preset as hard goals:
In Cruise Control, the following xref:main-goals[main optimization goals] are hard coded as hard goals:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
In Cruise Control, the following xref:main-goals[main optimization goals] are hard coded as hard goals:
In Cruise Control, the following xref:main-goals[main optimization goals] are hard goals:

Signed-off-by: Kyle Liberti <kliberti@redhat.com>
Copy link
Contributor

@PaulRMellor PaulRMellor left a comment

Choose a reason for hiding this comment

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

Thanks Kyle. I can see why the original text was leading to the misunderstanding.
I've made a few suggestions

kyguy and others added 5 commits February 13, 2024 08:30
Co-authored-by: PaulRMellor <47596553+PaulRMellor@users.noreply.github.com>
Signed-off-by: Kyle Liberti <kliberti@redhat.com>
Co-authored-by: PaulRMellor <47596553+PaulRMellor@users.noreply.github.com>
Signed-off-by: Kyle Liberti <kliberti@redhat.com>
Co-authored-by: PaulRMellor <47596553+PaulRMellor@users.noreply.github.com>
Signed-off-by: Kyle Liberti <kliberti@redhat.com>
Co-authored-by: PaulRMellor <47596553+PaulRMellor@users.noreply.github.com>
Signed-off-by: Kyle Liberti <kliberti@redhat.com>
Co-authored-by: PaulRMellor <47596553+PaulRMellor@users.noreply.github.com>
Signed-off-by: Kyle Liberti <kliberti@redhat.com>
Copy link
Member

@scholzj scholzj left a comment

Choose a reason for hiding this comment

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

Should be reviewed by SMEs (@ppatierno)

Copy link
Contributor

@fvaleri fvaleri left a comment

Choose a reason for hiding this comment

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

Much better. Thanks.

Copy link
Member

@ppatierno ppatierno left a comment

Choose a reason for hiding this comment

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

LGTM

kyguy and others added 2 commits February 14, 2024 08:34
Co-authored-by: PaulRMellor <47596553+PaulRMellor@users.noreply.github.com>
Signed-off-by: Kyle Liberti <kliberti@redhat.com>
Co-authored-by: PaulRMellor <47596553+PaulRMellor@users.noreply.github.com>
Signed-off-by: Kyle Liberti <kliberti@redhat.com>
@scholzj scholzj merged commit 20b7499 into strimzi:main Feb 14, 2024
13 checks passed
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.

None yet

6 participants