Skip to content
This repository has been archived by the owner on Jun 19, 2024. It is now read-only.

switch to using Deployments rather than DeploymentConfig on OpenShift when not using ImageStreams #1042

Closed
jstrachan opened this issue Sep 7, 2017 · 14 comments
Assignees
Milestone

Comments

@jstrachan
Copy link
Contributor

DeploymentConfig's are quite a bit worse to use than Deployments due to the extra deploy pods that sometimes timeout/fail, stay around, add confusion - whereas Deployments seem pretty rock solid! I've never seen a Deployment ever not deploy or timeout - whereas I've seen that happen lots with DCs.

So I wonder should we avoid DeploymentConfigs when not using ImageStreams on OpenShift? I guess we could make it a flag so folks could decide to keep DCs if they really want; but I'd have thought sticking with standard kubernetes resources OOTB sounds a nicer default (providing folks are not using ImageStreams)

@jstrachan jstrachan added this to the high priority milestone Sep 7, 2017
@rhuss
Copy link
Contributor

rhuss commented Sep 10, 2017

Isn't there more to DeploymentConfigs than only support for image stream ? E.g. like various, image stream independent, triggers for a redeployment ? Are triggers supports by plain Deployment's too ? (couldn't find anything like this in the API)

@hrishin
Copy link
Member

hrishin commented Sep 18, 2017

@jstrachan any comments?

@jstrachan
Copy link
Contributor Author

I don't see much value in a DeploymentConfig if not using ImageStreams; the DC related triggers only really relate to image streams. Plus there's lots of negatives - they require a pod each time a deployment happens which uses up quota & adds lots of confusion to users looking at pods on the CLI or in a console - plus they timeout unlike Deployments; we've also seen them behave odd at times ;).

In general I'm thinking fabric8-maven-plugin should always use DeploymentConfigs if using ImageStreams; but if not we should probably default to Deployments - or at least have a flag to switch either way.

I guess we can debate which side of the fence the OOTB should be if not using ImageStreams; but we should definitely allow developers to pick either Deployments or DeploymentConfigs for sure.

e.g. if you are targeting something like OpenShift Online - then just switching to Deployments will use less quota for your app with no real loss of functionality plus it'll be using standard kubernetes resources too & be less confusing if folks look at the pods.

@rhuss
Copy link
Contributor

rhuss commented Sep 18, 2017

@jstrachan fyi, DC triggers also work with plain Docker images, i.e. OpenShift periodically polls the Docker registry for updates (that's how we use it in Syndesis). Also, DCs can trigger on config changes (again not sure how plain Deployments react here). So I think DCs have a value even when not used with image streams.

Agreed, that the deployment pod can be a pita, especially when tight quotas apply like on OpenShift online (we also struggled with that, that certain deployments failed, because too much happened in parallel and so only 4 of 7 deployments succeed because of quota restrictions).

We should allow all sort of combinations via configuration, with some sane defaults. Still tend for the OpenShift case to DCs as default (with being overwritable to use plain Deployments).

@jstrachan
Copy link
Contributor Author

jstrachan commented Sep 18, 2017

@rhuss the docker DC triggers are only useful if you rebuild images and use latest right? If you are using specific image labels in your DC/Deployment then the DC image triggers have no point as you have to change the DC/Deployment anyway to use a different image label? As you're not really supposed to recreate images with the same label (other than latest).

Plain Deployments redeploy on changes to the Deployment (e.g. to change from 1.2.3 to 1.2.4 of the image version) so they work the same as 'config triggers' AFAIK

@rhuss
Copy link
Contributor

rhuss commented Sep 18, 2017

correct, only for 'latest' such a detection works (or when you overwrite a Docker tag, what you shouldn't do, of course). But for sharing current, snapshot work via a registry still useful imo.

@jstrachan
Copy link
Contributor Author

I guess thats another check - if folks are using 'latest' for the docker image then DC has some value - otherwise its got negative value.

FWIW in the fabric8 ecosystem we never use latest - which is fairly hard to reason about - and we tend to use concrete versions; they are so much easier to reason about. Even when generating temporary images for PRs so we can test them (e.g. like these comments) we use separate labels for each commit on a PR so you can easily test differences more easily.

e.g. see the comments from @fabric8cd on this PR
fabric8-ui/fabric8-ui#1978

we generate a docker image for each commit on a PR which can be tested on any docker-compatible enviroment - then we also run the UI using this PR image at the given URL (or run system tests using the image etc). Note that the UI for each PR is taken down after the PR merges so those links to the UI won't work any more on that PR - but you'll hopefully get the idea of how you can use those links during a PR review/approve/test

@hrishin
Copy link
Member

hrishin commented Oct 27, 2017

@jstrachan forgive me for naive question, but how to check if build is using image stream or docker stuff.
My assumption is fabric8.build.strategy flag.

@jstrachan
Copy link
Contributor Author

jstrachan commented Oct 27, 2017

Yeah - the fabric8:build goal decides whether to use an image stream or not - and an image stream YAML is generated.

I guess to keep things simple (as the fabric8:resource goal is often ran independently of the fabric8:build) we could just have a mode to default to DeploymentConfig on OpenShift but have a property/flag we could enable to default to always using Deployment - which would then fail if users tried to use an ImageStream

rohanKanojia added a commit to rohanKanojia/fabric8-maven-plugin that referenced this issue Oct 30, 2017
Switch to using Deployments rather than DeploymentConfig on OpenShift when not using ImageStreams
@rohanKanojia
Copy link
Member

rohanKanojia commented Oct 30, 2017

@jstrachan, @hrishin : Hi, I'm working on this issue. But i have a doubt; If we prefer Deployments rather than DeploymentConfig in the build strategy, the image is not getting resolved. I'm getting error :
Image pull back off.
But in case of DeploymentConfig it seems to work fine.
My question is: After building the image, what changes we need to accommodate in the build section as well? Or is it fine to have it in deployment for now?

@hrishin hrishin closed this as completed Nov 6, 2017
@hrishin hrishin reopened this Nov 6, 2017
@rohanKanojia
Copy link
Member

@jstrachan : Please share your thoughts on this.

@jstrachan
Copy link
Contributor Author

@rohanKanojia what kind of project are you trying this on? Is it using an image stream or not. So long as there is a docker image available (try docker pull foo on the CLI to check) - then you should be able to refer to that image in a Deployment just fine - afterall thats how fabric8 works on kubernetes

@rohanKanojia
Copy link
Member

rohanKanojia commented Nov 13, 2017

@jstrachan : I'm using samples/spring-boot project in fmp's code directory. And while building i'm using mvn -Dfabric8.build.strategy=docker clean install , i think that doesn't use image stream

rohanKanojia added a commit to rohanKanojia/fabric8-maven-plugin that referenced this issue Nov 15, 2017
Switch to using Deployments rather than DeploymentConfig on OpenShift when not using ImageStreams
@jstrachan
Copy link
Contributor Author

I wonder if you use -Dfabric8.mode=kubernetes if it will disable the use of ImageStreams

@hrishin hrishin modified the milestones: high priority, Sprint 141 Nov 22, 2017
@hrishin hrishin assigned hrishin and rohanKanojia and unassigned hrishin Nov 22, 2017
rohanKanojia added a commit to rohanKanojia/fabric8-maven-plugin that referenced this issue Nov 26, 2017
Switch to using Deployments rather than DeploymentConfig on OpenShift when not using ImageStreams
kameshsampath added a commit to kameshsampath/fabric8-maven-plugin that referenced this issue Dec 1, 2017
 - basic working version with sidecars, initcontainers added to app
 - added profile called "servicemesh" which need to be added to fmp config to enable this istio enrichers

WIP:
 - UT cases
 - failing with OpenShift, as Openshift deployment unable to lookup the image ??? related to fabric8io#1042
 - IT cases
@hrishin hrishin closed this as completed in 61d0d81 Dec 4, 2017
hrishin pushed a commit that referenced this issue Dec 4, 2017
Fixes #1042 Switch to using Deployments rather than DeploymentConfig on OpenShift when not using ImageStreams
kameshsampath added a commit to kameshsampath/fabric8-maven-plugin that referenced this issue Dec 4, 2017
 - basic working version with sidecars, initcontainers added to app
 - added profile called "servicemesh" which need to be added to fmp config to enable this istio enrichers

WIP:
 - UT cases
 - failing with OpenShift, as Openshift deployment unable to lookup the image ??? related to fabric8io#1042
 - IT cases
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants