[features] migrate npm feature #481
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Codecov Report
@@ Coverage Diff @@
## main #481 +/- ##
=======================================
Coverage 59.68% 59.68%
=======================================
Files 3 3
Lines 129 129
=======================================
Hits 77 77
Misses 40 40
Partials 12 12
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report at Codecov.
|
| @@ -59,13 +65,18 @@ func (impl *volumeManagerImpl) AddVolumeWithMergeFunc(volume *corev1.Volume, vol | |||
| return nil | |||
| } | |||
|
|
|||
| func (impl *volumeManagerImpl) AddVolumeToContainerWithMergeFunc(volume *corev1.Volume, volumeMount *corev1.VolumeMount, containerName commonv1.AgentContainerName, volumeMergeFunc VolumeMergeFunction, volumeMountMergeFunc VolumeMountMergeFunction) error { | |||
| func (impl *volumeManagerImpl) AddVolumeToContainersWithMergeFunc(volume *corev1.Volume, volumeMount *corev1.VolumeMount, containerNames []commonv1.AgentContainerName, volumeMergeFunc VolumeMergeFunction, volumeMountMergeFunc VolumeMountMergeFunction) error { | |||
There was a problem hiding this comment.
A solution should be to call the function AddVolumeToContainerWithMergeFunc() in a loop on containerNames. It will not be optimum in term of tentative to add the Volume, but it can reduce the code
There was a problem hiding this comment.
yea i thought about that, but in the worst case scenario it would loop through the impl.podTmpl.Spec.Containers 4 x 5 times..
we could add the return into the if block which would help a bit. do you prefer this route?
update: will implement the way you suggest
There was a problem hiding this comment.
we can keep the function as it is too 👍
| @@ -59,13 +65,18 @@ func (impl *volumeManagerImpl) AddVolumeWithMergeFunc(volume *corev1.Volume, vol | |||
| return nil | |||
| } | |||
|
|
|||
| func (impl *volumeManagerImpl) AddVolumeToContainerWithMergeFunc(volume *corev1.Volume, volumeMount *corev1.VolumeMount, containerName commonv1.AgentContainerName, volumeMergeFunc VolumeMergeFunction, volumeMountMergeFunc VolumeMountMergeFunction) error { | |||
| func (impl *volumeManagerImpl) AddVolumeToContainersWithMergeFunc(volume *corev1.Volume, volumeMount *corev1.VolumeMount, containerNames []commonv1.AgentContainerName, volumeMergeFunc VolumeMergeFunction, volumeMountMergeFunc VolumeMountMergeFunction) error { | |||
There was a problem hiding this comment.
we can keep the function as it is too 👍
* [features] migrate npm feature
What does this PR do?
Migrate NPM feature to new framework
Adjust VolumeManager to handle case when more than one but fewer than all containers need a volumeMount
Motivation
What inspired you to submit this pull request?
Additional Notes
Anything else we should know when reviewing?
Describe your test plan
Write there any instructions and details you may have to test your PR.