-
Notifications
You must be signed in to change notification settings - Fork 23
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
Load local image in local-deploy #192
Conversation
Makefile
Outdated
# The following "if" statement is still part of the "deploy" recipe. It couldn't be indented with tabs because it will | ||
# be considered as part of the recipe. It could be indented with spaces but I find it confusing. | ||
# https://stackoverflow.com/a/28720186/2749989 |
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.
From @eranra: This comment is obvious and should be removed.
Makefile
Outdated
@@ -148,9 +148,19 @@ push-image: build-image ## Push latest image | |||
|
|||
# note: to deploy with custom image tag use: DOCKER_TAG=test make deploy | |||
.PHONY: deploy | |||
deploy: ## Deploy the image | |||
deploy: build-image ## Deploy the 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.
The deploy
target is a dependency not only of local-deploy
but also of ocp-deploy
. Hence, it shouldn't load anything to kind and shouldn't depend on build-image
.
It doesn't support input from STDIN
contrib/kubernetes/deployment.yaml
Outdated
@@ -23,7 +23,7 @@ spec: | |||
- containerPort: 6343 | |||
- containerPort: 2055 | |||
- containerPort: 2056 | |||
imagePullPolicy: Always | |||
imagePullPolicy: Never |
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 should be set for Never
only for local-deployment.
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.
@ronensc maybe "IfNotPresent" fits both use-cases ... and actually if the code for OCP like in here https://cookbook.openshift.org/image-registry-and-image-streams/how-do-i-import-an-image-from-an-external-image.html works then you can keep the Never
approach and add the code for OCP deployment to also load an image ... I really prefer the second, but it does require a bit of more work to check that this works also for OCP.
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 tried loading the FLP image into an OCP cluster and then started a pod with that image
$ oc import-image quay.io/netobserv/flowlogs-pipeline:latest --confirm
$ oc run mypod --image=quay.io/netobserv/flowlogs-pipeline:latest --image-pull-policy=Never
But the pod couldn't find the image
$ oc get po
NAME READY STATUS RESTARTS AGE
mypod 0/1 ErrImageNeverPull 0 4s
Also, AFAIU, image streams are an OpenShift feature and there is no import-image
equivalent in kubectl
.
Do we want to introduce a new oc
command to our Makefile?
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.
@ronensc I was looking on https://medium.com/@adilsonbna/importing-an-external-docker-image-into-red-hat-openshift-repository-c25894cd3199
This seems to solve this ... but this is also not trivial and makes things even a bit more complicated
So, what I suggest is a keep things as is and move to IfNotPresent
mode ... this should work for both.
Also, small remark in the makefile of OCP that states that this uses the latest version from quay.io
Hope this makes sense to you
We might improve in one of the future PR's
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.
@jotak @mariomac @jpinsonneau @OlivierCazade if any of you guys know how to push an image from local docker into OCP (above links are almost working, but not really :-) ))) we can use that to improve the process of ocp-deploy
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.
Looks very good to me
When deployed on KinD, the image is pre-pushed to KinD from the local registry. When deployed on OCP, OCP will pull the image from quay.io.
Solves #148