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

Add combine script, to combine two manifest lists and push the result #871

Merged
merged 1 commit into from
Aug 10, 2021

Conversation

imjasonh
Copy link
Member

@imjasonh imjasonh commented Aug 2, 2021

Changes

This will be used to build a combined base image from our current
distroless/static base image, and a Windows base image, to produce a
base image that supports both.

This will, in turn, provide a base image for Tekton images so they can
be built with multi-OS support, in addition to their existing
multi-arch(-but-only-Linux) support today.

Ref tektoncd/pipeline#182

Submitter Checklist

These are the criteria that every PR should meet, please check them off as you
review them:

See the contribution guide
for more details.

/kind feature

@tekton-robot tekton-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. kind/feature Categorizes issue or PR as related to a new feature. labels Aug 2, 2021
Copy link
Member

@wlynch wlynch left a comment

Choose a reason for hiding this comment

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

Neat! Just some minor things, but otherwise LGTM!

Bonus points if you can figure out a way to test this while mocking out the remote calls. Maybe pull out the merging into a tested func?

cmd/combine/main.go Show resolved Hide resolved
cmd/combine/main.go Show resolved Hide resolved
cmd/combine/main.go Outdated Show resolved Hide resolved
cmd/combine/main.go Show resolved Hide resolved
cmd/combine/main.go Outdated Show resolved Hide resolved
cmd/combine/main.go Outdated Show resolved Hide resolved
cmd/combine/main.go Outdated Show resolved Hide resolved
This will be used to build a combined base image from our current
distroless/static base image, and a Windows base image, to produce a
base image that supports both.

This will, in turn, provide a base image for Tekton images so they can
be built with multi-OS support, in addition to their existing
multi-arch(-but-only-Linux) support today.
@wlynch
Copy link
Member

wlynch commented Aug 3, 2021

/lgtm
/approve

@tekton-robot tekton-robot added lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Aug 3, 2021
Copy link
Member

@vdemeester vdemeester left a comment

Choose a reason for hiding this comment

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

/test .*

cmd/combine/README.md Show resolved Hide resolved
@vdemeester
Copy link
Member

/test plumbing-yamllint

1 similar comment
@dibyom
Copy link
Member

dibyom commented Aug 5, 2021

/test plumbing-yamllint

@bobcatfish
Copy link
Contributor

Don't mind me, just trying something out:

/run try-out-tekton

@bobcatfish
Copy link
Contributor

hahaha well that sure didnt work - sorry for the noise @imjasonh ... now im not sure how to remove that check 😅

@bobcatfish
Copy link
Contributor

I forced it to be successful 🙏 sorry again for the noise

bobcatfish added a commit to bobcatfish/plumbing that referenced this pull request Aug 9, 2021
Two years ago I was playing around with Prow's tekton support as a way
to dogfood pipelines and I created this config to run a hello world type
pipeline on the plumbing repo
(tektoncd#66). Today while looking for
uses of PipelineResources to update to use the experimental Pipeline to
TaskRun custom task instead, I found this. It no longer runs
successfully (just added noise to
tektoncd#871)
and I don't think there's any reason to keep it around.
bobcatfish added a commit to bobcatfish/plumbing that referenced this pull request Aug 9, 2021
Two years ago I was playing around with Prow's tekton support as a way
to dogfood pipelines and I created this config to run a hello world type
pipeline on the plumbing repo
(tektoncd#66). Today while looking for
uses of PipelineResources to update to use the experimental Pipeline to
TaskRun custom task instead, I found this. It no longer runs
successfully (just added noise to
tektoncd#871)
and I don't think there's any reason to keep it around.
@vdemeester
Copy link
Member

/test plumbing-yamllint

@ghost
Copy link

ghost commented Aug 10, 2021

One more try to bump the yamllint task:

/test plumbing-yamllint

@ghost
Copy link

ghost commented Aug 10, 2021

/approve

@ghost
Copy link

ghost commented Aug 10, 2021

/approve cancel

@tekton-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: vdemeester, wlynch

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@jerop
Copy link
Member

jerop commented Aug 10, 2021

/test plumbing-yamllint

@jerop
Copy link
Member

jerop commented Aug 10, 2021

dropping lgtm to unblock tide

/lgtm cancel

@tekton-robot tekton-robot removed the lgtm Indicates that a PR is ready to be merged. label Aug 10, 2021
tekton-robot pushed a commit that referenced this pull request Aug 10, 2021
Two years ago I was playing around with Prow's tekton support as a way
to dogfood pipelines and I created this config to run a hello world type
pipeline on the plumbing repo
(#66). Today while looking for
uses of PipelineResources to update to use the experimental Pipeline to
TaskRun custom task instead, I found this. It no longer runs
successfully (just added noise to
#871)
and I don't think there's any reason to keep it around.
@jerop
Copy link
Member

jerop commented Aug 10, 2021

re-lgtming now that tide is unblocked and plumbing-yamllint has passed

/lgtm

@tekton-robot tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Aug 10, 2021
@jerop
Copy link
Member

jerop commented Aug 10, 2021

/test plumbing-yamllint

@tekton-robot tekton-robot merged commit 7245486 into tektoncd:main Aug 10, 2021
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/feature Categorizes issue or PR as related to a new feature. lgtm Indicates that a PR is ready to be merged. 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.

7 participants