-
Notifications
You must be signed in to change notification settings - Fork 116
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 more buildv1 features to the roadmap for vetting wrt buildv2 #41
add more buildv1 features to the roadmap for vetting wrt buildv2 #41
Conversation
Hi @gabemontero. Thanks for your PR. I'm waiting for a redhat-developer member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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. |
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 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 |
@@ -166,9 +166,24 @@ spec: | |||
| Private Builder Image Registry | ⚪️ | | | | |||
| Runtime Base Image | ⚪️ | | | | |||
| Binary builds | | | | | |||
| Image pull policy | | | | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would envision this being something buildv2 does solely by leveraging tekton
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Opened #46
@@ -166,9 +166,24 @@ spec: | |||
| Private Builder Image Registry | ⚪️ | | | | |||
| Runtime Base Image | ⚪️ | | | | |||
| Binary builds | | | | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there are some valuable lessons learned, especially in light of recent build v1 customer activity, as devex has discussed internally along with @bparees, that would should engage with @sbose78 's team on when they are ready to start on this.
Not sure if there is common library logic from buildv1 to share, especially since buildv2 may want to go down a different path than the tar to the build container path buildv1 employs today.
@adambkaplan - initial reaction, this feels like consulting work on our end vs. R&D and/or provided code on our end, but at least warrants discussion on our end
@@ -166,9 +166,24 @@ spec: | |||
| Private Builder Image Registry | ⚪️ | | | | |||
| Runtime Base Image | ⚪️ | | | | |||
| Binary builds | | | | | |||
| Image pull policy | | | | | |||
| Inject source via config map | | | | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would envision this being something buildv2 does solely by leveraging tekton
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Opened #48
@@ -166,9 +166,24 @@ spec: | |||
| Private Builder Image Registry | ⚪️ | | | | |||
| Runtime Base Image | ⚪️ | | | | |||
| Binary builds | | | | | |||
| Image pull policy | | | | | |||
| Inject source via config map | | | | | |||
| Inject source via secret | | | | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would envision this being something buildv2 does solely by leveraging tekton
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See #48 as well
| Image pull policy | | | | | ||
| Inject source via config map | | | | | ||
| Inject source via secret | | | | | ||
| Inject source via image | | | | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would envision this being something buildv2 does solely by leveraging tekton
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Opened #49
| Inject source via config map | | | | | ||
| Inject source via secret | | | | | ||
| Inject source via image | | | | | ||
| Set image label/ens | | | | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would envision this being something buildv2 does solely by leveraging tekton
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is setting labels on the output? what's ens? environment variables? if so env variables would be inputs right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah this needs to change to Set specified labels on output image
... the ens
was a leftover I thought I had deleted prior to branch push / PR creation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
opened #50
| Inject source via secret | | | | | ||
| Inject source via image | | | | | ||
| Set image label/ens | | | | | ||
| Triggers (SCMs) | | | | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://github.com/tektoncd/triggers already provides this, with more features already IMO, than what we have in buildv1
So I would envision this being something buildv2 does solely by leveraging tekton
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
opened #51
| Inject source via image | | | | | ||
| Set image label/ens | | | | | ||
| Triggers (SCMs) | | | | | ||
| Triggers (imagestream)| | | | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I envision devex changes for this @adambkaplan @siamaksade @bparees, updating our existing image change controller and the existing annotation it has for generic k8s object, to include support for both buildv2 and tekton
Certainly we can discuss the pros/cons with @sbose78 of them creating their own controller for buildv2 and this, but IMO the former idea would cost substantially less
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| Set image label/ens | | | | | ||
| Triggers (SCMs) | | | | | ||
| Triggers (imagestream)| | | | | ||
| Pruning of builds | | | | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is suppose to be a tekton feature (though if it has landed I have missed it)
perhaps an opportunity for either DEVEX or APPSVC to contribute upstream @adambkaplan @siamaksade @bparees @sbose78
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| Triggers (SCMs) | | | | | ||
| Triggers (imagestream)| | | | | ||
| Pruning of builds | | | | | ||
| Cancelling of builds | | | | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
believe this is already in tekton .... build v2 would just need to leverage
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| Triggers (imagestream)| | | | | ||
| Pruning of builds | | | | | ||
| Cancelling of builds | | | | | ||
| Chaining of builds | | | | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
doable via tekton today .... just whether buildv2 encapsulates this in the API like buildv1 did
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
buildv1 didn't really encapsulate it. it just allowed input content to come from an existing image, which you've already covered, and for the push of one image to trigger the build of another, which you've arguably already covered also w/ imagestream triggers
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah that's right ... thanks for the clarification. I'll remove this from this table, or a subsequent candidates table if we go down that path, and will refrain from opening an issue for discussion, per that last ask from @siamaksade
| Triggers (imagestream)| | | | | ||
| Pruning of builds | | | | | ||
| Cancelling of builds | | | | | ||
| Chaining of builds | | | | | ||
| Image Caching | | | | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@adambkaplan as discussed previously IMO DEVEX should either facilitate this with the existing stories or have subsequent ones that make sure our solution is consumable via the buildah
binary so buildv2 can leverage
| Image Caching | | | | | ||
| Max duration / TO | | | | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think tekton already has this ... if not, this should driven upstream by devtools or devexp
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| Image Caching | | | | | ||
| Max duration / TO | | | | | ||
| Parallel vs. serial execution | | | | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
again, tekton has ... buildv2 leverages via that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| Image Caching | | | | | ||
| Max duration / TO | | | | | ||
| Parallel vs. serial execution | | | | | ||
| ImageStreams support | | | | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@adambkaplan another tracking story for devex for consulting and/or provide code libs for this ... perhaps stage this later on after buildv2 matures a little
| Image Caching | | | | | ||
| Max duration / TO | | | | | ||
| Parallel vs. serial execution | | | | | ||
| ImageStreams support | | | | | ||
| Entitlements | | | | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@adambkaplan IMO same story as image cache ... the current CSI plugin choice for mutating annotated pods should work for buildv2 like buildv1 .... then it is a question of any OCM or openshift/builder image changes for buildv1 being pulled into the buildv2 controller
| ImageStreams support | | | | | ||
| Entitlements | | | | | ||
| Global Proxy support | | | | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@adambkaplan another story for us ... either something like obu
grabs the CAs. and injects them as part of the tekton task prior to calling the image builder binary, or we consult buildv2 on updating their controller to create config maps that get auto injected
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| ImageStreams support | | | | | ||
| Entitlements | | | | | ||
| Global Proxy support | | | | | ||
| Mirror support | | | | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same basic notions as with global proxy @adambkaplan
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| ImageStreams support | | | | | ||
| Entitlements | | | | | ||
| Global Proxy support | | | | | ||
| Mirror support | | | | | ||
| new-app/build & odo ? | | | | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is an interesting one (and I still have a question mark here) .... perhaps first a broad discussion with @sbose78 , @adambkaplan, @bparees, @siamaksade, and myself just the assess the current landscape
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While this list of features is valuable for comparison, I'm not sure it is accurate to project this list as the feature set that Build v2 will be addressing. Could we keep this list separate from the roadmap and as a feature comparison between Build v1 and v2? To serve as a source for cherry-picking and adding features to Build v2 as needed and as we get feedback on it |
i view this list as the critical features from builds v1 that we would want to see in builds v2 because they have proven valuable to our users and use cases, it is not an exhaustive list of things builds v1 supports. but by all means if you want to make the case that one or more of them is unnecessary let's have that discussion sooner rather than later. |
I understand that they were critical for build v1. However we need to revisit what the use-case was at the time, validate if it is still a relevant use-case now, if users are solving it in other ways, and if they are aligned with the goals of builds v2. Some I can see immediately that are still relevant like global proxy and some have doubts for example chaining builds. So I my suggestions is to discuss them one by one and make a decision on adding to the roadmap . |
as i said,
as i noted in the PR, there actually is no explicit support(api) for chained builds in buildsv1, "chained builds" is a capability that is achieved by the combination of other features (image input, image triggering), both of which i think we can agree are features we do need in builds v2. so yes, we can remove that from the list, but not because it's not important anymore in v2, but because it wasn't a thing in v1 either. (and tekton provides equivalent functionality with its stages) |
What is the use-case for image input? or for configmap and secrets? Triggering is also something I'd like to revisit given that pipelines already provide that capability. Same goes for Parallel vs. serial execution. |
"i have an image with existing dependency binaries, tools, etc that i need to use during my build. I do not want to/can not to layer on top of that existing image"
i need to control build behavior on a per buildconfig basis, i do not want to put this config in my git repo because the repo is used by many buildconfigs.
i have content i don't want to put in my github repo but i need to use it during a build (such as maven credentials) |
yes... tekton provides it. and builds v2 needs to expose it. that doesn't mean buildsv2 has to reimplement it. |
@bparees I don't think this PR is a good format to discuss these items. Makes is very difficult to follow. @gabemontero could you add a separate issue for each of those features to discuss separately? |
@siamaksade - my assumption was that github issues and/or jiras would get opened to discuss these items in detail as @sbose78 team got to the point where they were ready to take them on. And that his table here was a super concise and easy to consume report of what had been tackled to date and what might get tackled in the future. Or at least considered and then either taken on or dropped. Creating separate issues is fine, and I will do that momentarily. But to NOT have some level of correlation with this table would be unfortunate, as one then has to start querying github for open features, perhaps with certain labels, etc. etc. to get "the report". I at least am not a big fan of github's capabilities for this type of querying. But I may be in the minority here. How about this slight adjustment to your request:
Thoughts ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK I've opened issues for all the items I added to the current table @siamaksade
Do please let me know about my suggestion to have some form of "candidate" items in a table on this page vs. having to query for issues in github.
@@ -166,9 +166,24 @@ spec: | |||
| Private Builder Image Registry | ⚪️ | | | | |||
| Runtime Base Image | ⚪️ | | | | |||
| Binary builds | | | | | |||
| Image pull policy | | | | | |||
| Inject source via config map | | | | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Opened #48
@@ -166,9 +166,24 @@ spec: | |||
| Private Builder Image Registry | ⚪️ | | | | |||
| Runtime Base Image | ⚪️ | | | | |||
| Binary builds | | | | | |||
| Image pull policy | | | | | |||
| Inject source via config map | | | | | |||
| Inject source via secret | | | | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See #48 as well
| Image pull policy | | | | | ||
| Inject source via config map | | | | | ||
| Inject source via secret | | | | | ||
| Inject source via image | | | | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Opened #49
| Inject source via config map | | | | | ||
| Inject source via secret | | | | | ||
| Inject source via image | | | | | ||
| Set image label/ens | | | | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
opened #50
| Inject source via secret | | | | | ||
| Inject source via image | | | | | ||
| Set image label/ens | | | | | ||
| Triggers (SCMs) | | | | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
opened #51
| Image Caching | | | | | ||
| Max duration / TO | | | | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| Image Caching | | | | | ||
| Max duration / TO | | | | | ||
| Parallel vs. serial execution | | | | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| ImageStreams support | | | | | ||
| Entitlements | | | | | ||
| Global Proxy support | | | | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| ImageStreams support | | | | | ||
| Entitlements | | | | | ||
| Global Proxy support | | | | | ||
| Mirror support | | | | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| ImageStreams support | | | | | ||
| Entitlements | | | | | ||
| Global Proxy support | | | | | ||
| Mirror support | | | | | ||
| new-app/build & odo ? | | | | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @gabemontero I suggest we close this PR and work off the GitHub issues. The ones that get determined are relevant in the context of builds v2, should get added back into the features table. Merging this PR implies incorrectly that all these features will be added at some point. |
As part of closing the PR @siamaksade I'd like a label .... something like I don't have auth in this repo to do this. @sbose78 can you craft a label for us? thanks everyone |
/assign @sbose78
Hey @sbose78
Finally got around to submitting updates to the roadmap you started based on some of our initial conversations.
My updates here are based on a feature compare between openshift build v1 and tekton that I did last fall.
As we progress through buildv2 we should look at each of these and decide whether we
At the time of my evaluation last fall, the conclusion wrt to tekton and each of these was either
a) it was already implemented at the tekton API level
b) could be done via the openness of the tekton API, where pods/containers are exposed along with supporting artifacts like secrets/configmap/volumes/pvcs etc., and "more work" on our end as needed.
c) or at least was on the tekton "roadmap" in as much as upstream features were open
And of course aside from discussion here, we can zero in on this on an upcoming build v2 sync call as you like.
From the openshift devex end, as I've mentioned before, I see us trying to facilitate your consumption of such features via work like
obu
or similar utilities, repo refactoring, inclusion of the build v2 scenarios in upcoming design work, whatever we come up with that makes sense and is containable.@pweil- @adambkaplan @siamaksade @bparees ... FYI and of course chime in as needed