-
Notifications
You must be signed in to change notification settings - Fork 165
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 support for platform API 0.7 and 0.8 #889
Conversation
Signed-off-by: Sambhav Kothari <skothari44@bloomberg.net>
fcb833b
to
efc2914
Compare
Signed-off-by: Sambhav Kothari <skothari44@bloomberg.net>
efc2914
to
72d7708
Compare
Codecov Report
@@ Coverage Diff @@
## main #889 +/- ##
==========================================
+ Coverage 68.08% 68.25% +0.17%
==========================================
Files 116 116
Lines 5164 5192 +28
==========================================
+ Hits 3516 3544 +28
Misses 1287 1287
Partials 361 361
Continue to review full report at Codecov.
|
Signed-off-by: Sambhav Kothari <skothari44@bloomberg.net>
b939a66
to
4536ee3
Compare
Signed-off-by: Sambhav Kothari <skothari44@bloomberg.net>
4536ee3
to
6091b51
Compare
do we want to include the |
It's not needed for the way kpack builds things. It can just use the defaults (run image from stack.toml and stack.toml location as |
Do we want to also relax our mixin validation logic within that platform api? |
Added an additional issue to track mixin validation relaxation: #891 |
}, | ||
}, | ||
ifWindows(buildContext.os(), addNetworkWaitLauncherVolume(), useNetworkWaitLauncher(dnsProbeHost))..., | ||
func() corev1.Container { |
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'd like to explore moving the instantiation of these containers within the pod spec (with the accepted possibility of some duplication). The step pattern was originally added to allow all the containers to be explicitly provided in the pod spec with some logic.
I think we can have something like:
if platformAPILessThan07 {
step(detect...)
step(analyze..)
}else {
step(analyze...)
step(detect..)
}
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.
Happy to explore that in a future refactor though :)
This PR adds support for 0.7 and 0.8.
Resolves #886