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.

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

[WIP] Remove Artifacts Package #6096

Closed
wants to merge 1 commit into from

Conversation

JeromeJu
Copy link
Member

@JeromeJu JeromeJu commented Feb 1, 2023

Changes

This commit cleans up the artifacts package.
It prepares for the removal storage resources to mitigate dependencies.

/kind cleanup

Submitter Checklist

As the author of this PR, please check off the items in this checklist:

  • Has Docs included if any changes are user facing
  • Has Tests included if any functionality added or changed
  • Follows the commit message standard
  • Meets the Tekton contributor standards (including
    functionality, content, code)
  • Has a kind label. You can add one by adding a comment on this PR that contains /kind <type>. Valid types are bug, cleanup, design, documentation, feature, flake, misc, question, tep
  • Release notes block below has been updated with any user facing changes (API changes, bug fixes, changes requiring upgrade notices or deprecation warnings)
  • Release notes contains the string "action required" if the change requires additional action from users switching to the new release

Release Notes

NONE

@tekton-robot
Copy link
Collaborator

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@tekton-robot tekton-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. release-note-none Denotes a PR that doesnt merit a release note. labels Feb 1, 2023
@tekton-robot tekton-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 1, 2023
@tekton-robot
Copy link
Collaborator

@JeromeJu: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@tekton-robot tekton-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Feb 1, 2023
@tekton-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please assign dibyom after the PR has been reviewed.
You can assign the PR to them by writing /assign @dibyom in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

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

@tekton-robot
Copy link
Collaborator

The following Tekton test failed:

Test name Commit Details Required Rerun command
check-pr-has-kind-label 87d8ce0 link true /test check-pr-has-kind-label

@JeromeJu
Copy link
Member Author

JeromeJu commented Feb 1, 2023

I have been trying to cleanup the artifacts first before moving the entire chunk of storage resources.

Some questions I encountered during the cleanup:

  • It feels that the output_resources paths are affected by the removal of such setup for pvc, shall we remove them along with the artifacts package?

@lbernick
Copy link
Member

lbernick commented Feb 1, 2023

I think artifacts are probably required for the storage resource to work correctly (although haven't looked into this thoroughly), so removing artifacts before removing storage resources would put us in an unreleasable state.

@JeromeJu
Copy link
Member Author

JeromeJu commented Feb 1, 2023

I think artifacts are probably required for the storage resource to work correctly (although haven't looked into this thoroughly), so removing artifacts before removing storage resources would put us in an unreleasable state.

Thanks for the instant replies. If this is not preferred then let's remove both storage resources and artifacts in one shot.

@JeromeJu JeromeJu closed this Feb 1, 2023
@afrittoli
Copy link
Member

Given that this is already a large PR, I'm not sure we should bundle it with more changes, but I agree with @lbernick that we must first delete the remaining PipelineResources and then this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. release-note-none Denotes a PR that doesnt merit a release note. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants