Proposal: Support to Specify Extra Volumes and Volume Mounts for Karmada Control Plane Components

[jabellard]

What type of PR is this?

/kind design

What this PR does / why we need it:

• Details are contained in the proposal.

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

NONE

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 33.86%. Comparing base (3a4861f) to head (2dfd11d).
Report is 286 commits behind head on master.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #5276      +/-   ##
==========================================
+ Coverage   28.24%   33.86%   +5.62%     
==========================================
  Files         632      643      +11     
  Lines       43753    44500     +747     
==========================================
+ Hits        12360    15072    +2712     
+ Misses      30492    28280    -2212     
- Partials      901     1148     +247     

Flag	Coverage Δ
unittests	33.86% <ø> (+5.62%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[jabellard]

@RainbowMango , thanks for the constructive feedback. I just pushed some changes to address your comments, so I think this should be ready to go. Currently working on the implementation PR and will push that soon.

[RainbowMango]

/assign

[RainbowMango]

Hi @jabellard I'm looking at the field names, both fields are prefixed with Extra.

I can understand why you named it with extra, that is we already have some hardcoded volumes and mounts, like:

  volumes:
  - name: apiserver-cert
    secret:
      defaultMode: 420
      secretName: karmada-demo-cert
  - name: etcd-cert
    secret:
      defaultMode: 420
      secretName: karmada-demo-etcd-cert

and

    volumeMounts:
    - mountPath: /etc/karmada/pki
      name: apiserver-cert
      readOnly: true
    - mountPath: /etc/etcd/pki
      name: etcd-cert
      readOnly: true

I have a feeling that a little bit of context is missing here.

Maybe we should go without extra or add more comments on that. What do you think?

[RainbowMango]

By the way, we have the same feeling on ExtraArgs just on above of the proposed fields. From the users's perspective, they might not know what kind of args are already set.

[jabellard]

Thanks for sharing that perspective. I do agree with you that a user creating the CR will lack context into what will be set by default if it's not documented. For volumes and volumes mounts, by default, the code will add volumes and volume mounts for certs as that's a base requirement for a functioning control plane. As you mentioned above, we can remediate the lack of context from a user's perspective by adding more documentation to clearly communicate what is automatically handled by the operator for a base setup, and how those new fields can be used the further customize that setup. Do you see that as sensible way forward?

[RainbowMango]

Yes, I agree, I guess you mean adding more documentation is more comments on that.

[jabellard]

Yes. Just pushed some changes for that.

[RainbowMango]

/lgtm
/approve
Thanks.

[karmada-bot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: RainbowMango

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

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