Conversation
tomkennedy513
left a comment
There was a problem hiding this comment.
Should we blow up if someone tries to create a builder for a windows stack? Along the same line, should we blow up if a build tries to happen on an existing builder that is windows based?
related discussion #1366 Signed-off-by: Tom Kennedy <tom.kennedy@broadcom.com>
3b2cd8b to
27ad1b4
Compare
We have to make a few more changes to use them directly. @natalieparellano has most of a pr adding support for that if you want to help drive it over the line |
|
I believe, after merging this PR we can close the following issues #1131 and #1333, right? @tomkennedy513 |
Signed-off-by: Tom Kennedy <kennedy513@gmail.com>
Added an error and test case if the stack is windows |
chenbh
left a comment
There was a problem hiding this comment.
We also need an RFC PR for this, mainly so that we can mark https://github.com/buildpacks-community/kpack/blob/main/rfcs/0003-windows-images.md as superseded/reverted
Good call |
b9ad5b3 to
8ffa690
Compare
- also clean up unused code Signed-off-by: Tom Kennedy <kennedy513@gmail.com>
8ffa690 to
93342d1
Compare
chenbh
left a comment
There was a problem hiding this comment.
What do you think about removing the OS field from the BuilderStatus? I'm on the fence as I don't seem much harm either way
kpack/pkg/apis/build/v1alpha2/builder_types.go
Lines 70 to 80 in f5c8a87
Ya I left it because it didn't seem like it caused harm, but if it's only ever going to be Linux, then it probably can be removed |
@chenbh One issue is that it's technically a breaking change in the api if we remove it |
Yes, but we're breaking functionality by removing windows anyways. Although I don't care too deeply about it and am happy to leave it up to you |
|
@chenbh Going to merge this as-is once the rfc is approved and we can revisit later to see if we should remove the os field from the builder status |
With windows support removed from the lifecycle we have no way to build windows builders.
rfc #1837
related discussion #1366