Skip to content

Conversation

@raukadah
Copy link
Contributor

No description provided.

@openshift-ci openshift-ci bot requested review from fmount and gibizer November 11, 2025 09:11
raukadah added a commit to raukadah/watcher-operator that referenced this pull request Nov 11, 2025
Signed-off-by: Chandan Kumar (raukadah) <[email protected]>
Copy link
Contributor

@fmount fmount left a comment

Choose a reason for hiding this comment

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

/approve
/lgtm

@openshift-ci
Copy link

openshift-ci bot commented Nov 12, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: fmount

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:

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

@openshift-merge-bot openshift-merge-bot bot merged commit 810d396 into openstack-k8s-operators:main Nov 12, 2025
4 checks passed
Comment on lines +224 to +231
run_bg ${BASH_ALIASES[os]} optimize goal list '>' "$WATCHER_PATH"/goal_list
run_bg ${BASH_ALIASES[os]} optimize strategy list '>' "$WATCHER_PATH"/strategy_list
run_bg ${BASH_ALIASES[os]} optimize audittemplate list '>' "$WATCHER_PATH"/audittemplate_list
run_bg ${BASH_ALIASES[os]} optimize audit list '>' "$WATCHER_PATH"/audit_list
run_bg ${BASH_ALIASES[os]} optimize actionplan list '>' "$WATCHER_PATH"/actionplan_list
run_bg ${BASH_ALIASES[os]} optimize action list '>' "$WATCHER_PATH"/action_list
run_bg ${BASH_ALIASES[os]} optimize service list '>' "$WATCHER_PATH"/watcher_service_list
run_bg ${BASH_ALIASES[os]} optimize datamodel list '>' "$WATCHER_PATH"/watcher_datamodel_list

Choose a reason for hiding this comment

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

this is not correct we shoudl not be collecting the datamodel actions, audits or action plans

the goals and stragies would only hcange if they added there own plugins for thos so the only thing i thikn we shoudl likely keep is the service list

Suggested change
run_bg ${BASH_ALIASES[os]} optimize goal list '>' "$WATCHER_PATH"/goal_list
run_bg ${BASH_ALIASES[os]} optimize strategy list '>' "$WATCHER_PATH"/strategy_list
run_bg ${BASH_ALIASES[os]} optimize audittemplate list '>' "$WATCHER_PATH"/audittemplate_list
run_bg ${BASH_ALIASES[os]} optimize audit list '>' "$WATCHER_PATH"/audit_list
run_bg ${BASH_ALIASES[os]} optimize actionplan list '>' "$WATCHER_PATH"/actionplan_list
run_bg ${BASH_ALIASES[os]} optimize action list '>' "$WATCHER_PATH"/action_list
run_bg ${BASH_ALIASES[os]} optimize service list '>' "$WATCHER_PATH"/watcher_service_list
run_bg ${BASH_ALIASES[os]} optimize datamodel list '>' "$WATCHER_PATH"/watcher_datamodel_list
run_bg ${BASH_ALIASES[os]} optimize service list '>' "$WATCHER_PATH"/watcher_service_list

if you look at the nova version you can see we intentiolly do not collelct the flavor or instances as we are not ment to collect the workload info via must gather

collection the action plans, actions autit templates and audits re all "workload info" in the context of watcher.

i maybe coudl see collectin the list of audittempelates but honeslty i dotn think that will be useful for debuging.

Choose a reason for hiding this comment

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

I agree that probably the only useful part will be the service list and maybe audits (i.e. to see the parameters for questions like "my audit X is not doing what I'd expect) although for those case it will probably easier to ask for specific commands.

We can also keep the list of goals and strategies just to check that the deployment of watcher went fine.

Said that, I'd say the only bad side of it is extra space usage, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@SeanMooney @amoralej you are correct, collecting all info does help. #116 now collects goal, strategy and services list.

Choose a reason for hiding this comment

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

the goals and stragies i think is fine

the probelm with the workload information is w eshoudl not be collecting data that might be sensivite and things liek the parmaters pass to an audit may be.

certainly i know we have to be careful with collecting ips and hostnames in general, the service list i think is ok but i think the datamodel is too far at least by default.

we could support it via a flag but i strongly suspect we would be much better off requesting this info explicit if necessary. if that happen often we can expand the list but for now lets keep this pretty simple and short.

@SeanMooney
Copy link

sigh we shoudl revert this and do it proeprly or fix it

before we merge this i also woudl have liked to see this run end to end in ci to review the output.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants