fix(CNV-28185): add required labels

[codingben]

What this PR does / why we need it:

Add required labels:

• "app.kubernetes.io/managed-by"
• "app.kubernetes.io/version"
• "app.kubernetes.io/component"
• "app.kubernetes.io/part-of"

That are required by a specific resource:

• ssp-operator-metrics

Jira-Url: https://issues.redhat.com/browse/CNV-28185

Release note:

Add required labels to SSP operator metrics

[codingben]

Hi @akrejcir @0xFelix, can we merge at least what we have now? other SSP objects probably deployed by HCO and labels should be set by HCO.

[akrejcir]

Please update the commit message and PR's description to reflect the current change.

[codingben]

Please update the commit message and PR's description to reflect the current change.

Done.

[codingben]

@0xFelix Can you please re-review it again? If there is no part-of value, it'll be empty.

[0xFelix]

There is still a CI failure.

[0xFelix]

Can you please add functional tests too?

[ksimon1]

/retest

[codingben]

/hold

There is a failure in the new functional test that I've added, I'll try to run it locally again.

[codingben]

@akrejcir Thanks for spotting it! :) Can we just change the managed-by label value of service object to ssp-operator-metrics instead of ssp-operator? It would also not need to modify the functional test.

I'm not sure. The meaning of the label is: The tool being used to manage the operation of an application https://kubernetes.io/docs/concepts/overview/working-with-objects/common-labels/#labels

There is no tool called ssp-operator-metrics. But the example in the documentation is helm, so maybe we are using the label incorrectly.

I think there is a design issue from the beginning. Why metrics is part of ssp-operator? why ssp-operator deploys Metrics Rules and many other resources?

From what I see, the service object ssp-operator-metrics is controlled by services_controller, and all other resources are controlled by ssp_controller. Maybe it would be okay to have the managed-by label value as ssp-operator-services or ssp-operator-metrics.

What others think, @lyarwood and @0xFelix? your opinion is very much appreciated. :)

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

[0xFelix]

@codingben Depends on what we want to achieve.

If we want absolutely no resources to be left when the SSP CR is removed then we have another bug. If we want just objects from ssp_controller to be gone then we could change the label value.

IMO we should not leave any resources when the SSP CR is removed, so IMO we have a bug?

[akrejcir]

The metrics service resource does not depend on SSP CR. It needs to be present when the ssp-operator is running, otherwise metrics will not be picked up by prometheus.
It should have owner reference pointing to the ssp-operator Deployment, so it will be removed when operator is removed.

[0xFelix]

How do we ensure the resources are cleaned up when removing the operator then?

[akrejcir]

I've edited the above comment, but you were faster.

[0xFelix]

@akrejcir @codingben Then it sounds to me like we should have different managed-by labels for the operands.

[akrejcir]

I'm ok with a different label. I'm not sure where are the labels used.

[0xFelix]

AFAIU they are used by HCO.

[codingben]

In the failed tests I see some infra failures, e.g. network/timeout errors.

/retest

[codingben]

/retest

[codingben]

/retest

[codingben]

/retest

[codingben]

@akrejcir @0xFelix @ksimon1 @lyarwood Hi, finally CI passed. Can you please merge it?

[akrejcir]

/lgtm

[0xFelix]

/approve

Thanks!

[kubevirt-bot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 0xFelix

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 [0xFelix]

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

[codingben]

/unhold

[akrejcir]

/cherry-pick release-v0.18

[kubevirt-bot]

@akrejcir: new pull request created: #658

In response to this:

/cherry-pick release-v0.18

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.