Skip to content

Add V1 Pipeline Golang structs#5219

Merged
tekton-robot merged 1 commit intotektoncd:mainfrom
JeromeJu:4986-v1-pipeline
Aug 8, 2022
Merged

Add V1 Pipeline Golang structs#5219
tekton-robot merged 1 commit intotektoncd:mainfrom
JeromeJu:4986-v1-pipeline

Conversation

@JeromeJu
Copy link
Member

@JeromeJu JeromeJu commented Jul 26, 2022

Changes

This commit adds structs and validation for v1 Pipeline, including when expressions,
resultRef. This is copied from v1beta1 with the following exceptions:

  • omitted PipelineResources
  • omitted Bundle and ClusterTask fields
  • change PipelineTask whenExpressions Go field name from to When
    due to api naming violation

Part of #4986
/kind misc

Submitter Checklist

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

  • [n/a] 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)
  • [n/a] 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. do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Jul 26, 2022
@tekton-robot tekton-robot requested review from jerop and lbernick July 26, 2022 17:53
@tekton-robot tekton-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Jul 26, 2022
@lbernick lbernick self-assigned this Jul 26, 2022
@lbernick
Copy link
Member

We do need to keep our alpha fields around (including matrix); they should just stay behind an alpha feature flag. the v1 TEP is the source of truth for what should be in the v1 api

@tekton-robot tekton-robot added the ok-to-test Indicates a non-member PR verified by an org member that is safe to test. label Jul 27, 2022
@tekton-robot
Copy link
Collaborator

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/apis/pipeline/v1/param_types.go 97.1% 94.5% -2.6
pkg/apis/pipeline/v1/pipeline_defaults.go Do not exist 0.0%
pkg/apis/pipeline/v1/pipeline_types.go Do not exist 94.5%
pkg/apis/pipeline/v1/pipeline_validation.go Do not exist 100.0%
pkg/apis/pipeline/v1/resultref.go Do not exist 87.3%
pkg/apis/pipeline/v1/status_validation.go Do not exist 100.0%
pkg/apis/pipeline/v1/taskref_validation.go Do not exist 0.0%
pkg/apis/pipeline/v1/when_types.go Do not exist 100.0%
pkg/apis/pipeline/v1/when_validation.go Do not exist 100.0%

@tekton-robot
Copy link
Collaborator

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/apis/pipeline/v1/param_types.go 97.1% 94.5% -2.6
pkg/apis/pipeline/v1/pipeline_defaults.go Do not exist 0.0%
pkg/apis/pipeline/v1/pipeline_types.go Do not exist 94.5%
pkg/apis/pipeline/v1/pipeline_validation.go Do not exist 100.0%
pkg/apis/pipeline/v1/resultref.go Do not exist 87.3%
pkg/apis/pipeline/v1/status_validation.go Do not exist 100.0%
pkg/apis/pipeline/v1/taskref_validation.go Do not exist 0.0%
pkg/apis/pipeline/v1/when_types.go Do not exist 100.0%
pkg/apis/pipeline/v1/when_validation.go Do not exist 100.0%

@tekton-robot
Copy link
Collaborator

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/apis/pipeline/v1/param_types.go 97.1% 94.5% -2.6
pkg/apis/pipeline/v1/pipeline_defaults.go Do not exist 0.0%
pkg/apis/pipeline/v1/pipeline_types.go Do not exist 94.5%
pkg/apis/pipeline/v1/pipeline_validation.go Do not exist 100.0%
pkg/apis/pipeline/v1/resultref.go Do not exist 87.3%
pkg/apis/pipeline/v1/status_validation.go Do not exist 100.0%
pkg/apis/pipeline/v1/taskref_validation.go Do not exist 0.0%
pkg/apis/pipeline/v1/when_types.go Do not exist 100.0%
pkg/apis/pipeline/v1/when_validation.go Do not exist 100.0%


// ValidateEmbeddedStatus checks that the embedded-status feature gate is set to the wantEmbeddedStatus value and,
// if not, returns an error stating which feature is dependent on the status and what the current status actually is.
func ValidateEmbeddedStatus(ctx context.Context, featureName, wantEmbeddedStatus string) *apis.FieldError {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this file shouldn't be needed for Pipeline (only relevant to PipelineRun)

Copy link
Member Author

@JeromeJu JeromeJu Jul 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that this is used in the call stack for validateMatrix, which calls the following func:
https://github.com/JeromeJu/pipeline/blob/5e7bc86121ed814b7066b57d5686b5338326abcd/pkg/apis/pipeline/v1/pipeline_types.go#L289

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it that makes sense! In v1, we will only allow this feature flag to be set to "minimal". I think this is OK to stay, but we should also add a similar validation function for PipelineRun (not in this PR) that prevents a v1 PipelineRun from being created unless the feature flag is set to "minimal".

Copy link
Member Author

@JeromeJu JeromeJu Jul 28, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Lee. I've noted this down and will be applying to the PipelineRun change which I'd also like to take after this PR and related work.
#4987

@lbernick
Copy link
Member

Shall we exclude the files generated by ./hack/update-codegen.sh under pkg/client?

No, these files are code generated by knative, which we need

@tekton-robot
Copy link
Collaborator

@JeromeJu: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

  • /test pull-tekton-pipeline-alpha-integration-tests
  • /test pull-tekton-pipeline-build-tests
  • /test pull-tekton-pipeline-integration-tests
  • /test tekton-pipeline-unit-tests

The following commands are available to trigger optional jobs:

  • /test pull-tekton-pipeline-go-coverage

Use /test all to run all jobs.

Details

In response to this:

/test check-pr-has-kind-label

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.

@JeromeJu JeromeJu changed the title [WIP] Add V1 Pipeline Golang structs Add V1 Pipeline Golang structs Jul 27, 2022
@tekton-robot tekton-robot added the kind/misc Categorizes issue or PR as a miscellaneuous one. label Jul 28, 2022
@tekton-robot
Copy link
Collaborator

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/apis/pipeline/v1/param_types.go 97.1% 94.5% -2.6
pkg/apis/pipeline/v1/pipeline_defaults.go Do not exist 0.0%
pkg/apis/pipeline/v1/pipeline_types.go Do not exist 93.1%
pkg/apis/pipeline/v1/pipeline_validation.go Do not exist 100.0%
pkg/apis/pipeline/v1/resultref.go Do not exist 72.7%
pkg/apis/pipeline/v1/status_validation.go Do not exist 100.0%
pkg/apis/pipeline/v1/taskref_validation.go Do not exist 0.0%
pkg/apis/pipeline/v1/when_types.go Do not exist 100.0%
pkg/apis/pipeline/v1/when_validation.go Do not exist 100.0%

@tekton-robot
Copy link
Collaborator

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/apis/pipeline/v1/param_types.go 97.1% 94.5% -2.6
pkg/apis/pipeline/v1/pipeline_defaults.go Do not exist 0.0%
pkg/apis/pipeline/v1/pipeline_types.go Do not exist 93.1%
pkg/apis/pipeline/v1/pipeline_validation.go Do not exist 100.0%
pkg/apis/pipeline/v1/resultref.go Do not exist 72.7%
pkg/apis/pipeline/v1/status_validation.go Do not exist 100.0%
pkg/apis/pipeline/v1/taskref_validation.go Do not exist 0.0%
pkg/apis/pipeline/v1/when_types.go Do not exist 100.0%
pkg/apis/pipeline/v1/when_validation.go Do not exist 100.0%

@tekton-robot
Copy link
Collaborator

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/apis/pipeline/v1/param_types.go 97.1% 94.5% -2.6
pkg/apis/pipeline/v1/pipeline_defaults.go Do not exist 100.0%
pkg/apis/pipeline/v1/pipeline_types.go Do not exist 93.1%
pkg/apis/pipeline/v1/pipeline_validation.go Do not exist 100.0%
pkg/apis/pipeline/v1/resultref.go Do not exist 72.7%
pkg/apis/pipeline/v1/status_validation.go Do not exist 100.0%
pkg/apis/pipeline/v1/taskref_validation.go Do not exist 100.0%
pkg/apis/pipeline/v1/when_types.go Do not exist 100.0%
pkg/apis/pipeline/v1/when_validation.go Do not exist 100.0%

@JeromeJu JeromeJu marked this pull request as ready for review July 28, 2022 23:37
@tekton-robot tekton-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 28, 2022
@tekton-robot tekton-robot requested review from abayer and imjasonh July 28, 2022 23:37
@tekton-robot
Copy link
Collaborator

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/apis/pipeline/v1/param_types.go 97.1% 96.3% -0.8
pkg/apis/pipeline/v1/pipeline_defaults.go Do not exist 100.0%
pkg/apis/pipeline/v1/pipeline_types.go Do not exist 95.1%
pkg/apis/pipeline/v1/pipeline_validation.go Do not exist 99.4%
pkg/apis/pipeline/v1/resultref.go Do not exist 98.2%
pkg/apis/pipeline/v1/status_validation.go Do not exist 100.0%
pkg/apis/pipeline/v1/taskref_validation.go Do not exist 100.0%
pkg/apis/pipeline/v1/when_types.go Do not exist 100.0%
pkg/apis/pipeline/v1/when_validation.go Do not exist 100.0%

@tekton-robot
Copy link
Collaborator

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/apis/pipeline/v1/param_types.go 97.1% 96.3% -0.8
pkg/apis/pipeline/v1/pipeline_defaults.go Do not exist 100.0%
pkg/apis/pipeline/v1/pipeline_types.go Do not exist 95.1%
pkg/apis/pipeline/v1/pipeline_validation.go Do not exist 99.4%
pkg/apis/pipeline/v1/resultref.go Do not exist 98.2%
pkg/apis/pipeline/v1/status_validation.go Do not exist 100.0%
pkg/apis/pipeline/v1/taskref_validation.go Do not exist 100.0%
pkg/apis/pipeline/v1/when_types.go Do not exist 100.0%
pkg/apis/pipeline/v1/when_validation.go Do not exist 100.0%

@tekton-robot
Copy link
Collaborator

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/apis/pipeline/v1/param_types.go 97.1% 96.3% -0.8
pkg/apis/pipeline/v1/pipeline_defaults.go Do not exist 100.0%
pkg/apis/pipeline/v1/pipeline_types.go Do not exist 95.1%
pkg/apis/pipeline/v1/pipeline_validation.go Do not exist 99.4%
pkg/apis/pipeline/v1/resultref.go Do not exist 98.2%
pkg/apis/pipeline/v1/status_validation.go Do not exist 100.0%
pkg/apis/pipeline/v1/taskref_validation.go Do not exist 100.0%
pkg/apis/pipeline/v1/when_types.go Do not exist 100.0%
pkg/apis/pipeline/v1/when_validation.go Do not exist 100.0%

@tekton-robot tekton-robot added release-note-none Denotes a PR that doesnt merit a release note. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Jul 29, 2022
@tekton-robot
Copy link
Collaborator

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/apis/pipeline/v1/param_types.go 97.1% 96.3% -0.8
pkg/apis/pipeline/v1/pipeline_defaults.go Do not exist 100.0%
pkg/apis/pipeline/v1/pipeline_types.go Do not exist 95.1%
pkg/apis/pipeline/v1/pipeline_validation.go Do not exist 99.4%
pkg/apis/pipeline/v1/resultref.go Do not exist 98.2%
pkg/apis/pipeline/v1/status_validation.go Do not exist 100.0%
pkg/apis/pipeline/v1/taskref_validation.go Do not exist 100.0%
pkg/apis/pipeline/v1/when_types.go Do not exist 100.0%
pkg/apis/pipeline/v1/when_validation.go Do not exist 100.0%

@tekton-robot
Copy link
Collaborator

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/apis/pipeline/v1/param_types.go 97.1% 96.3% -0.8
pkg/apis/pipeline/v1/pipeline_defaults.go Do not exist 100.0%
pkg/apis/pipeline/v1/pipeline_types.go Do not exist 95.1%
pkg/apis/pipeline/v1/pipeline_validation.go Do not exist 99.4%
pkg/apis/pipeline/v1/resultref.go Do not exist 98.2%
pkg/apis/pipeline/v1/status_validation.go Do not exist 100.0%
pkg/apis/pipeline/v1/taskref_validation.go Do not exist 100.0%
pkg/apis/pipeline/v1/when_types.go Do not exist 100.0%
pkg/apis/pipeline/v1/when_validation.go Do not exist 100.0%

@tekton-robot
Copy link
Collaborator

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/apis/pipeline/v1/param_types.go 97.1% 96.3% -0.8
pkg/apis/pipeline/v1/pipeline_defaults.go Do not exist 100.0%
pkg/apis/pipeline/v1/pipeline_types.go Do not exist 95.1%
pkg/apis/pipeline/v1/pipeline_validation.go Do not exist 99.4%
pkg/apis/pipeline/v1/resultref.go Do not exist 98.2%
pkg/apis/pipeline/v1/status_validation.go Do not exist 100.0%
pkg/apis/pipeline/v1/taskref_validation.go Do not exist 100.0%
pkg/apis/pipeline/v1/when_types.go Do not exist 100.0%
pkg/apis/pipeline/v1/when_validation.go Do not exist 100.0%

@tekton-robot
Copy link
Collaborator

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/apis/pipeline/v1/param_types.go 97.1% 96.3% -0.8
pkg/apis/pipeline/v1/pipeline_defaults.go Do not exist 100.0%
pkg/apis/pipeline/v1/pipeline_types.go Do not exist 95.1%
pkg/apis/pipeline/v1/pipeline_validation.go Do not exist 99.4%
pkg/apis/pipeline/v1/resultref.go Do not exist 98.2%
pkg/apis/pipeline/v1/status_validation.go Do not exist 100.0%
pkg/apis/pipeline/v1/taskref_validation.go Do not exist 100.0%
pkg/apis/pipeline/v1/when_types.go Do not exist 100.0%
pkg/apis/pipeline/v1/when_validation.go Do not exist 100.0%

Copy link
Member

@lbernick lbernick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Jerome! could you please:

  • update the PR description to specify that the Go field name for When was changed, not the API
  • add a commit message body
  • double check whether there have been any changes to pkg/apis affecting these files since you opened this PR, and make sure they're incorporated here?

@tekton-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: lbernick

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

The pull request process is described here

Details 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 tekton-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 2, 2022
This commit adds structs and validation for v1 Pipeline, including when expressions,
resultRef. This is copied from v1beta1 with the following exceptions:

- omitted PipelineResources
- omitted Bundle and ClusterTask fields
- change PipelineTask `whenExpressions` Go field name from to `When`
due to api naming violation
@tekton-robot
Copy link
Collaborator

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/apis/pipeline/v1/param_types.go 97.1% 96.3% -0.8
pkg/apis/pipeline/v1/pipeline_defaults.go Do not exist 100.0%
pkg/apis/pipeline/v1/pipeline_types.go Do not exist 95.1%
pkg/apis/pipeline/v1/pipeline_validation.go Do not exist 99.4%
pkg/apis/pipeline/v1/resultref.go Do not exist 98.2%
pkg/apis/pipeline/v1/status_validation.go Do not exist 100.0%
pkg/apis/pipeline/v1/taskref_validation.go Do not exist 100.0%
pkg/apis/pipeline/v1/when_types.go Do not exist 100.0%
pkg/apis/pipeline/v1/when_validation.go Do not exist 100.0%

@JeromeJu
Copy link
Member Author

JeromeJu commented Aug 2, 2022

Thanks Jerome! could you please:

  • update the PR description to specify that the Go field name for When was changed, not the API
  • add a commit message body
  • double check whether there have been any changes to pkg/apis affecting these files since you opened this PR, and make sure they're incorporated here?

Thanks Lee:) Updated.

@JeromeJu JeromeJu mentioned this pull request Aug 2, 2022
6 tasks
@jerop
Copy link
Member

jerop commented Aug 2, 2022

/assign

Copy link
Member

@jerop jerop left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@tekton-robot tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Aug 8, 2022
@JeromeJu
Copy link
Member Author

JeromeJu commented Aug 8, 2022

/retest

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. kind/misc Categorizes issue or PR as a miscellaneuous one. lgtm Indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. 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.

5 participants