Add support for repeated PVC-claim but using subPath in AA-validation #3099
Conversation
ghost
requested a review
from 
tekton-robot
added
release-note
tekton-robot
added
the
size/L
ghost
mentioned this pull request
|
The following is the coverage report on the affected files.
|
| // ValidateOnlyOnePVCIsUsed checks that a list of WorkspaceBinding uses only | ||
| // one persistent volume claim. This is useful to validate that WorkspaceBindings | ||
| // are compatible with affinity rules enforced by the AffinityAssistant. | ||
| func ValidateOnlyOnePVCIsUsed(wb []v1beta1.WorkspaceBinding) error { |
There was a problem hiding this comment.
nit: maybe add to the comment that this condition (multiple pvcs) is valid for WorkspaceBinding in a PipelineRun, but not for a TaskRun
There was a problem hiding this comment.
good thinking; I've updated the comment to include that detail.
There was a problem hiding this comment.
This is useful to validate that WorkspaceBindings
// in PipelineRuns are compatible with affinity rules enforced by the
// AffinityAssistant.
It's the other way around :) . We only call this function from taskrun.go because this condition is ok when declared in a PipelineRun
There was a problem hiding this comment.
Maybe
// This is only useful to validate that WorkspaceBindings
// in TaskRuns are compatible with affinity rules enforced by the
// AffinityAssistant.
There was a problem hiding this comment.
oops, hah, thanks I've corrected it with your wording.
tekton-robot
added
lgtm
|
The following is the coverage report on the affected files.
|
The validation for compatibility with the Affinity Assistant does not support the same PVC repeated, but using different subPaths. This patch adds support for this case and tests for the validation. Co-authored-by: Scott <sbws@google.com>
|
The following is the coverage report on the affected files.
|
|
Do we want this PR in the v0.16 release milestone? |
I think so, unless we want it in a patch. It should be in a v0.15.x also. |
|
/lgtm |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bobcatfish, jlpettersson 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 |
tekton-robot
added
the
approved
Changes
Picks up from #3091 and adds some tests for the validation. Original commit message:
The validation for compatibility with the Affinity Assistant does not
support the same PVC repeated, but using different subPaths.
This patch adds support for this case and tests for the validation.
Co-authored-by: Scott sbws@google.com
Closes #3085
Closes #3091
/kind bug
Submitter Checklist
These are the criteria that every PR should meet, please check them off as you
review them:
Release Notes