feat: Add podLabels to Helm chart

[bjw-s]

Describe what this PR does
It is already possible to define Pod annotations on the volsync controller Deployment. This PR makes it possible to set Pod labels as well.

Is there anything that requires special attention?

Related issues:

[openshift-ci]

Hi @bjw-s. Thanks for your PR.

I'm waiting for a backube member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

[tesshuflower]

Can you roll back this change? I don't want to modify the default version to v0.9.0 at this point

[bjw-s]

I'm fine doing that, I merely changed it because of that instruction This version number should be incremented each time you make changes to the chart following semVer.

I will submit a PR soon.

[bjw-s]

I've removed the chart version bump.
Looks like some formatter fixes were applied automatically, are they OK to keep?

[tesshuflower]

Thanks for that - you're right normally we would update the version, but we'll do it at the same time when we make a v0.9 release. I think the formatter fixes are probably ok.

[sonarcloud]

Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

[tesshuflower]

@JohnStrunk are you ok with this update to the helm charts?

[JohnStrunk]

Looks fine to me. Of note: this doesn't cover the mover pods, just the VolSync controller.
/ok-to-test
/approve

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bjw-s, JohnStrunk

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 [JohnStrunk]

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

[bjw-s]

Looks fine to me. Of note: this doesn't cover the mover pods, just the VolSync controller.

/ok-to-test

/approve

I would love to be able to set labels on the mover pods as well (to more easily target them with networkPolicies for example), but that would require changes to the Go code. I haven't dared to dig in to that yet 😅

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 67.0%. Comparing base (958da7c) to head (8c9920f).
Report is 466 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##            main   #1054     +/-   ##
=======================================
+ Coverage   66.9%   67.0%   +0.1%     
=======================================
  Files         55      55             
  Lines       7080    7080             
=======================================
+ Hits        4739    4748      +9     
+ Misses      2061    2055      -6     
+ Partials     280     277      -3     

see 1 file with indirect coverage changes

[tesshuflower]

Looks fine to me. Of note: this doesn't cover the mover pods, just the VolSync controller.
/ok-to-test
/approve

I would love to be able to set labels on the mover pods as well (to more easily target them with networkPolicies for example), but that would require changes to the Go code. I haven't dared to dig in to that yet 😅

@bjw-s FYI: We've just made a change for #707 which also included the ability to set podLabels for a mover. See PR: #1072

[tesshuflower]

/lgtm

[bjw-s]

@bjw-s FYI: We've just made a change for #707 which also included the ability to set podLabels for a mover. See PR: #1072

That's awesome! Thank you! 🙏

[bjw-s]

@tesshuflower I'm having some trouble figuring out where the test is failing exactly (and how it can be failing based on my changes 😅). Can you perhaps offer some guidance?

[tesshuflower]

Most likely not due to your change, sometimes we're still getting intermittent failures as these test clusters are pretty resource constrained.

/retest