Use wget for http/https package in downloader container #556
Conversation
jiangpengcheng
requested review from
nlu90,
freeznet and
a team
as code owners
freeznet
added
m/2023-01
type/enhancement
| @@ -511,6 +516,10 @@ func getLegacyDownloadCommand(downloadPath, componentPackage string, authProvide | |||
| func getDownloadCommand(downloadPath, componentPackage string, tlsProvided, authProvided bool, tlsConfig TLSConfig, | |||
| authConfig *v1alpha1.AuthConfig) []string { | |||
| var args []string | |||
| if hasHTTPPrefix(downloadPath) { | |||
| args = append(args, "wget", downloadPath, "-O", componentPackage) | |||
There was a problem hiding this comment.
do we need to check the availability of wget first, then call it to download?
There was a problem hiding this comment.
the pulsarctl image has wget, I think we can update the doc to inform users what a downloadContainer requires (currently, it requires pulsarctl and wget)
| @@ -511,6 +516,10 @@ func getLegacyDownloadCommand(downloadPath, componentPackage string, authProvide | |||
| func getDownloadCommand(downloadPath, componentPackage string, tlsProvided, authProvided bool, tlsConfig TLSConfig, | |||
| authConfig *v1alpha1.AuthConfig) []string { | |||
| var args []string | |||
| if hasHTTPPrefix(downloadPath) { | |||
There was a problem hiding this comment.
Another question: any chance to make this feature backport to getLegacyDownloadCommand?
There was a problem hiding this comment.
looks like our runtime image neither have wget nor curl, so there will be some compatibility issues if add this feature to getLegacyDownloadCommand:
- we can add
wgetorcurlto runtime images and add this feature - but we also need to deprecate old runtime images or check whether given runtime images have
wgetorcurlinstalled
I think there should be a general process for such a procedure(when we need to update runtime images):
- add
wgetorcurlor other required utils to runtime images and release new versions - add the new feature to FunctionMesh
- release FunctionMesh vx.y.z, and make an announcement that FunctionMesh vx.y.z is not compatible with runtime images older than va.b.c
- for user's custom runtime images, we list the requirements(what the image should contain)
HDYT? @freeznet @tpiperatgod @nlu90 ?
There was a problem hiding this comment.
This can be easily solved by Buildpacks, but we currently need a user-friendly image build workflow...
There was a problem hiding this comment.
are Buildpacks-related commits merged? or docs updated in functionmesh.io?
There was a problem hiding this comment.
has provided the docs and demos
Fixes #545
Motivation
For http/https packages, we don't need to talk with pulsar cluster to download them
Modifications
Use wget for http/https packages
Verifying this change
Make sure that the change passes the CI checks.
This change added tests and can be verified as follows:
Documentation
Check the box below.
Need to update docs?
doc-required(If you need help on updating docs, create a doc issue)
no-need-doc(Please explain why)
doc(If this PR contains doc changes)