-
Notifications
You must be signed in to change notification settings - Fork 522
Conversation
460ec5b
to
48890a2
Compare
/cc @ritazh |
70c4c46
to
ce2c837
Compare
Codecov Report
@@ Coverage Diff @@
## master #2936 +/- ##
=========================================
- Coverage 72.58% 72.1% -0.48%
=========================================
Files 141 141
Lines 25893 26251 +358
=========================================
+ Hits 18794 18929 +135
- Misses 6005 6228 +223
Partials 1094 1094 |
@@ -0,0 +1,287 @@ | |||
apiVersion: storage.k8s.io/v1beta1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because we're installing this by default and it's so much larger in terms of its yaml byte payload compared to keyvault-flexvol, we may run into cloud-init data limits. We'll want to test this with other data-heavy configurations (e.g., calico).
If we do find that this touches the limit we can schedule some cloud-init payload optimization refactor work, but that would become a blocker to this landing.
@ritazh FYI
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Going to test it now with calico enabled.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jackfrancis I was able to successfully deploy 1.16 cluster with csi-secrets-store
and calico
enabled. Any other heavy config that you are aware of we should test with?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for validating! I'm testing across the 1.17 matrix, will report back if any cluster configs complain about customData limits.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Validation passed, so we don't have to worry about this (for now!)
acff520
to
e1839b0
Compare
e1839b0
to
0145cc3
Compare
0145cc3
to
1bb6425
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: aramase, jackfrancis 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 |
Reason for Change:
keyvault-flexvolume
andcsi-secrets-store
addon enabled simultaneously, then will result in an errorkeyvault-flexvolume
addon enabled explicitly for 1.16 clusters, a warning message will be displayed andcsi-secrets-store
addon will not be enabled.csi-secrets-store
addon for < 1.16 clustersIssue Fixed:
fixes #2695
Requirements:
Notes: