Skip to content
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

Fixes #1130 Application redeployment is getting failed on OpenShift v3.7.0 #1176

Merged
merged 1 commit into from
Feb 12, 2018

Conversation

rohanKanojia
Copy link
Member

  • Added a workaround to deal with ImageTriggers.
  • Added flag fabric8.openshift.trimImageInContainerSpec which would trim
    image in container spec

#1130

@pepperbot
Copy link
Collaborator

SonarQube analysis reported 1 issue

  • CRITICAL 1 critical

Watch the comments in this conversation to review them.

@@ -50,7 +50,7 @@ public DeploymentOpenShiftConverter(PlatformMode mode, Long openshiftDeployTimeo
}

@Override
public HasMetadata convert(HasMetadata item) {
public HasMetadata convert(HasMetadata item, boolean trimImageInContainerSpec) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CRITICAL Refactor this method to reduce its Cognitive Complexity from 49 to the 15 allowed. rule

@mojsha
Copy link

mojsha commented Feb 8, 2018

@rohanKanojia
Can you add the possibility of setting automatic=false on the ImageTrigger? This would be much appreciated as we need the Image Trigger to resolve ImageStreamTag for the image, but do not want automatic re-deployment based on the ImageTrigger.

@rohanKanojia
Copy link
Member Author

@mojsha : Yes, I think it's possible. Could you please file an issue for that and assign it to me?

@mojsha
Copy link

mojsha commented Feb 8, 2018

@rohanKanojia: Done #1177, but I could not assign it to you.

Copy link
Member

@hrishin hrishin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good @rohanKanojia. But there is catch (may be wrong), we should call the setImage("") if its redeployment scenario. isnt it?

@rohanKanojia
Copy link
Member Author

@hrishin : I think that during the build, we're don't know whether it's deployment scenario or redeployment scenario. Fabric8 maven plugin does a PUT of deployment config, so deploy or redeploy it's the same thing. Here we're just avoiding the update in container image spec, which was triggering redeployments. ImageTrigger would automatically inject the image into it.

Copy link
Member

@hrishin hrishin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, Rohan!!
Could you please update the doc. with this flag description?

…enShift v3.7.0

+ Added a workaround to deal with ImageTriggers.
+ Added flag fabric8.openshift.trimImageInContainerSpec which would trim
  image in container spec
+ Updated documentation related to flag.
Copy link
Member

@hrishin hrishin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rohanKanojia looks good now.
Thank you!

@hrishin hrishin merged commit af0e677 into fabric8io:master Feb 12, 2018
@pradeepto
Copy link
Collaborator

@rohanKanojia @hrishin Did we run all required tests after this merge?

rohanKanojia added a commit to rohanKanojia/fabric8-maven-plugin that referenced this pull request Apr 10, 2019
…tainerSpec parameter to set image field empty

This paramter was introduced in fabric8io#1176 which stops
automatic redeployments in openshift by setting image field in DC empty.

I skipped this parameter during refactoring but now since it's causing redeployments on Openshift even
after setting fabric8.openshift.enableAutomatic=true; let's reintroduce this.
rohanKanojia added a commit to rohanKanojia/fabric8-maven-plugin that referenced this pull request Apr 11, 2019
…tainerSpec parameter to set image field empty

This paramter was introduced in fabric8io#1176 which stops
automatic redeployments in openshift by setting image field in DC empty.

I skipped this parameter during refactoring but now since it's causing redeployments on Openshift even
after setting fabric8.openshift.enableAutomatic=true; let's reintroduce this.
rohanKanojia added a commit to rohanKanojia/fabric8-maven-plugin that referenced this pull request Apr 11, 2019
…tainerSpec parameter to set image field empty

This paramter was introduced in fabric8io#1176 which stops
automatic redeployments in openshift by setting image field in DC empty.

I skipped this parameter during refactoring but now since it's causing redeployments on Openshift even
after setting fabric8.openshift.enableAutomatic=true; let's reintroduce this.
rohanKanojia added a commit that referenced this pull request Apr 16, 2019
…c parameter to set image field empty (#1618)

This paramter was introduced in #1176 which stops
automatic redeployments in openshift by setting image field in DC empty.

I skipped this parameter during refactoring but now since it's causing redeployments on Openshift even
after setting fabric8.openshift.enableAutomatic=true; let's reintroduce this.
@rohanKanojia rohanKanojia deleted the issue1130 branch September 23, 2021 10:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants