Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Refactor CreatePVCsForWorkspacesWithoutAffinityAssistant #6915

Closed
QuanZhang-William opened this issue Jul 10, 2023 · 1 comment · Fixed by #6921
Closed

Refactor CreatePVCsForWorkspacesWithoutAffinityAssistant #6915

QuanZhang-William opened this issue Jul 10, 2023 · 1 comment · Fixed by #6921
Assignees
@QuanZhang-William
Labels
kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt.

Comments

@QuanZhang-William
Copy link
Member

CreatePVCsForWorkspacesWithoutAffinityAssistant is current used to create PVCs from PipelineRuns's VolumeClaimTemplate.

As changed in #6741 and #6893, the PVC creation logic now depends on the AffinityAssistantBehavior. This change makes the current implementation of CreatePVCsForWorkspacesWithoutAffinityAssistant and createOrUpdateAffinityAssistantsPerAABehavior hard to understand.

We need to refactor CreatePVCsForWorkspacesWithoutAffinityAssistant and its usage to fit new new way of creating PVCs.
(Originally suggested by @lbernick at #6893 (comment))

/kind cleanup
@tekton-robot tekton-robot added the kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. label Jul 10, 2023
@QuanZhang-William
Copy link
Member Author

/assign

@QuanZhang-William QuanZhang-William mentioned this issue Jul 10, 2023
Merged
7 tasks
QuanZhang-William added a commit to QuanZhang-William/pipeline that referenced this issue Jul 11, 2023
@QuanZhang-William
[TEP-0135] Refactor CreatePVCsForWorkspaces
6b7e8a5 
Part of [tektoncd#6740][tektoncd#6740] and closes [tektoncd#6915].

This commit refactors the original `CreatePVCsForWorkspacesWithoutAffinityAssistant` (renamed to `CreatePVCFromVolumeClaimTemplate`) function
and its usages to improve readability since the PVC creation logic is now dependent on `AffinityAssistantBehavior`.

/kind cleanup

[tektoncd#6740]: tektoncd#6740
[tektoncd#6915]: tektoncd#6915
QuanZhang-William added a commit to QuanZhang-William/pipeline that referenced this issue Jul 11, 2023
@QuanZhang-William
[TEP-0135] Refactor CreatePVCsForWorkspaces
b84bdd8 
Part of [tektoncd#6740][tektoncd#6740] and closes [tektoncd#6915].

This commit refactors the original `CreatePVCsForWorkspacesWithoutAffinityAssistant` (renamed to `CreatePVCFromVolumeClaimTemplate`) function
and its usages to improve readability since the PVC creation logic is now dependent on `AffinityAssistantBehavior`.

/kind cleanup

[tektoncd#6740]: tektoncd#6740
[tektoncd#6915]: tektoncd#6915
QuanZhang-William added a commit to QuanZhang-William/pipeline that referenced this issue Jul 11, 2023
@QuanZhang-William
[TEP-0135] Refactor CreatePVCsForWorkspaces
84c18fc 
Part of [tektoncd#6740][tektoncd#6740] and closes [tektoncd#6915].

This commit refactors the original `CreatePVCsForWorkspacesWithoutAffinityAssistant` (renamed to `CreatePVCFromVolumeClaimTemplate`) function
and its usages to improve readability since the PVC creation logic is now dependent on `AffinityAssistantBehavior`.

/kind cleanup

[tektoncd#6740]: tektoncd#6740
[tektoncd#6915]: tektoncd#6915
QuanZhang-William added a commit to QuanZhang-William/pipeline that referenced this issue Jul 11, 2023
@QuanZhang-William
[TEP-0135] Refactor CreatePVCsForWorkspaces
d01c1be 
Part of [tektoncd#6740][tektoncd#6740] and closes [tektoncd#6915].

This commit refactors the original `CreatePVCsForWorkspacesWithoutAffinityAssistant` (renamed to `CreatePVCFromVolumeClaimTemplate`) function
and its usages to improve readability since the PVC creation logic is now dependent on `AffinityAssistantBehavior`.

There is no functionality change in the commit.

/kind cleanup

[tektoncd#6740]: tektoncd#6740
[tektoncd#6915]: tektoncd#6915
QuanZhang-William added a commit to QuanZhang-William/pipeline that referenced this issue Jul 11, 2023
@QuanZhang-William
[TEP-0135] Refactor CreatePVCsForWorkspaces
4ec9197 
Part of [tektoncd#6740][tektoncd#6740] and closes [tektoncd#6915].

This commit refactors the original `CreatePVCsForWorkspacesWithoutAffinityAssistant` (renamed to `CreatePVCFromVolumeClaimTemplate`) function
and its usages to improve readability since the PVC creation logic is now dependent on `AffinityAssistantBehavior`.

There is no functionality change in the commit.

/kind cleanup

[tektoncd#6740]: tektoncd#6740
[tektoncd#6915]: tektoncd#6915
@QuanZhang-William QuanZhang-William mentioned this issue Jul 12, 2023
Merged
7 tasks
QuanZhang-William added a commit to QuanZhang-William/pipeline that referenced this issue Jul 12, 2023
@QuanZhang-William
[TEP-0135] Refactor CreatePVCsForWorkspaces
70fd19d 
Part of [tektoncd#6740][tektoncd#6740] and closes [tektoncd#6915].

Prior to this commit, the `createOrUpdateAffinityAssistantsAndPVCs` function attempts to create all Affinity Assistant StatefulSets and returns aggregated errors.
This could result in time and resource waste when executing a pipelinerun that will fail. This commit updates it to "fail fast" strategy
where the function is returned as soon as the first error is encountered.

This commit also refactors the original `CreatePVCsForWorkspacesWithoutAffinityAssistant` (renamed to `CreatePVCFromVolumeClaimTemplate`) function
and its usages to improve readability since the PVC creation logic is now dependent on `AffinityAssistantBehavior`.

/kind feature

[tektoncd#6740]: tektoncd#6740
[tektoncd#6915]: tektoncd#6915
QuanZhang-William added a commit to QuanZhang-William/pipeline that referenced this issue Jul 12, 2023
@QuanZhang-William
[TEP-0135] Refactor CreatePVCsForWorkspaces
1b38911 
Part of [tektoncd#6740][tektoncd#6740] and closes [tektoncd#6915].

Prior to this commit, the `createOrUpdateAffinityAssistantsAndPVCs` function attempts to create all Affinity Assistant StatefulSets and returns aggregated errors.
This could result in time and resource waste when executing a pipelinerun that will fail. This commit updates it to "fail fast" strategy
where the function is returned as soon as the first error is encountered.

This commit also refactors the original `CreatePVCsForWorkspacesWithoutAffinityAssistant` (renamed to `CreatePVCFromVolumeClaimTemplate`) function
and its usages to improve readability since the PVC creation logic is now dependent on `AffinityAssistantBehavior`.

/kind feature

[tektoncd#6740]: tektoncd#6740
[tektoncd#6915]: tektoncd#6915
QuanZhang-William added a commit to QuanZhang-William/pipeline that referenced this issue Jul 24, 2023
@QuanZhang-William
[TEP-0135] Refactor CreatePVCsForWorkspaces
e50a7fb 
Part of [tektoncd#6740][tektoncd#6740] and closes [tektoncd#6915].

Prior to this commit, the `createOrUpdateAffinityAssistantsAndPVCs` function attempts to create all Affinity Assistant StatefulSets and returns aggregated errors.
This could result in time and resource waste when executing a pipelinerun that will fail. This commit updates it to "fail fast" strategy
where the function is returned as soon as the first error is encountered.

This commit also refactors the original `CreatePVCsForWorkspacesWithoutAffinityAssistant` (renamed to `CreatePVCFromVolumeClaimTemplate`) function
and its usages to improve readability since the PVC creation logic is now dependent on `AffinityAssistantBehavior`.

/kind feature

[tektoncd#6740]: tektoncd#6740
[tektoncd#6915]: tektoncd#6915
QuanZhang-William added a commit to QuanZhang-William/pipeline that referenced this issue Jul 25, 2023
@QuanZhang-William
[TEP-0135] Refactor CreatePVCsForWorkspaces
f130104 
Part of [tektoncd#6740][tektoncd#6740] and closes [tektoncd#6915].

Prior to this commit, the `createOrUpdateAffinityAssistantsAndPVCs` function attempts to create all Affinity Assistant StatefulSets and returns aggregated errors.
This could result in time and resource waste when executing a pipelinerun that will fail. This commit updates it to "fail fast" strategy
where the function is returned as soon as the first error is encountered.

This commit also refactors the original `CreatePVCsForWorkspacesWithoutAffinityAssistant` (renamed to `CreatePVCFromVolumeClaimTemplate`) function
and its usages to improve readability since the PVC creation logic is now dependent on `AffinityAssistantBehavior`.

/kind feature

[tektoncd#6740]: tektoncd#6740
[tektoncd#6915]: tektoncd#6915
tekton-robot pushed a commit that referenced this issue Jul 26, 2023
@QuanZhang-William @tekton-robot
[TEP-0135] Refactor CreatePVCsForWorkspaces
1eef4db 
Part of [#6740][#6740] and closes [#6915].

Prior to this commit, the `createOrUpdateAffinityAssistantsAndPVCs` function attempts to create all Affinity Assistant StatefulSets and returns aggregated errors.
This could result in time and resource waste when executing a pipelinerun that will fail. This commit updates it to "fail fast" strategy
where the function is returned as soon as the first error is encountered.

This commit also refactors the original `CreatePVCsForWorkspacesWithoutAffinityAssistant` (renamed to `CreatePVCFromVolumeClaimTemplate`) function
and its usages to improve readability since the PVC creation logic is now dependent on `AffinityAssistantBehavior`.

/kind feature

[#6740]: #6740
[#6915]: #6915
@QuanZhang-William QuanZhang-William mentioned this issue Jul 27, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@QuanZhang-William QuanZhang-William

Labels
kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[TEP-0135] Refactor CreatePVCsForWorkspaces QuanZhang-William/pipeline
2 participants
@QuanZhang-William @tekton-robot