Skip to content

Commit

Permalink
Fixes fabric8io#1177 Minor changes to ImageTrigger generation
Browse files Browse the repository at this point in the history
Added fabric8.openshift.setImageTriggerAutomatic flag which would
be able to enable/disable automatic deployments.
  • Loading branch information
rohanKanojia committed Feb 13, 2018
1 parent 2c40eac commit 8203161
Show file tree
Hide file tree
Showing 8 changed files with 18 additions and 8 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ After this we will switch probably to real [Semantic Versioning 2.0.0](http://se
* Fix 1130: Added flag fabric8.openshift.trimImageInContainerSpec which would set the container image reference to "", this is done to handle weird
behavior of Openshift 3.7 in which subsequent rollouts lead to ImagePullErr.
* Feature 1174: ImageStreams use local lookup policy by default to simplify usage of Deployment or StatefulSet resources on Openshift
* Fix 1177: Added flag fabric8.openshift.setImageTriggerAutomatic which would be able to enable/disable automatic deployments whenever there is new image
generated.

###3.5.34
* Feature 1003: Added suspend option to remote debugging
Expand Down
6 changes: 5 additions & 1 deletion doc/src/main/asciidoc/inc/goals/build/_fabric8-resource.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -185,4 +185,8 @@ Resource goal also validates the generated resource descriptors using API specif
| *fabric8.openshift.trimImageInContainerSpec*
| If value is set to `true` then it would set the container image reference to "", this is done to handle weird behavior of Openshift 3.7 in which subsequent rollouts lead to ImagePullErr
| false
|===

| *fabric8.openshift.setImageTriggerAutomatic*
| If the value is set to `false` then automatic deployments would be disabled.
| true
|===
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ public DeploymentConfigOpenShiftConverter(Long openshiftDeployTimeoutSeconds) {
}

@Override
public HasMetadata convert(HasMetadata item, boolean trimImageInContainerSpec) {
public HasMetadata convert(HasMetadata item, boolean trimImageInContainerSpec, boolean setImageTriggerAutomatic) {
if (item instanceof DeploymentConfig) {
DeploymentConfig resource = (DeploymentConfig) item;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public DeploymentOpenShiftConverter(PlatformMode mode, Long openshiftDeployTimeo
}

@Override
public HasMetadata convert(HasMetadata item, boolean trimImageInContainerSpec) {
public HasMetadata convert(HasMetadata item, boolean trimImageInContainerSpec, boolean setImageTriggerAutomatic) {
Deployment resource = (Deployment) item;
DeploymentConfigBuilder builder = new DeploymentConfigBuilder();
builder.withMetadata(resource.getMetadata());
Expand Down Expand Up @@ -119,10 +119,11 @@ public HasMetadata convert(HasMetadata item, boolean trimImageInContainerSpec) {
specBuilder.addNewTrigger()
.withType("ImageChange")
.withNewImageChangeParams()
.withAutomatic(true)
.withAutomatic(setImageTriggerAutomatic)
.withNewFrom()
.withKind("ImageStreamTag")
.withName(image.getSimpleName() + ":" + tag)
.withNamespace(image.getUser())
.endFrom()
.withContainerNames(containerName)
.endImageChangeParams()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,6 @@
*/
public interface KubernetesToOpenShiftConverter {

HasMetadata convert(HasMetadata item, boolean trimImageInContainerSpec);
HasMetadata convert(HasMetadata item, boolean trimImageInContainerSpec, boolean setImageTriggerAutomatic);

}
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
*/
public class NamespaceOpenShiftConverter implements KubernetesToOpenShiftConverter {
@Override
public HasMetadata convert(HasMetadata item, boolean trimImageInContainerSpec) {
public HasMetadata convert(HasMetadata item, boolean trimImageInContainerSpec, boolean setImageTriggerAutomatic) {
return new ProjectRequestBuilder().withMetadata(item.getMetadata()).build();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
*/
public class ReplicSetOpenShiftConverter implements KubernetesToOpenShiftConverter {
@Override
public HasMetadata convert(HasMetadata item, boolean trimImageInContainerSpec) {
public HasMetadata convert(HasMetadata item, boolean trimImageInContainerSpec, boolean setImageTriggerAutomatic) {
ReplicaSet resource = (ReplicaSet) item;
ReplicationControllerBuilder builder = new ReplicationControllerBuilder();
builder.withMetadata(resource.getMetadata());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,9 @@ public class ResourceMojo extends AbstractResourceMojo {
@Parameter(property = "fabric8.openshift.trimImageInContainerSpec", defaultValue = "false")
private Boolean trimImageInContainerSpec;

@Parameter(property = "fabric8.openshift.setImageTriggerAutomatic", defaultValue = "true")
private Boolean setImageTriggerAutomatic;

@Parameter(property = "kompose.dir", defaultValue = "${user.home}/.kompose/bin")
private File komposeBinDir;

Expand Down Expand Up @@ -757,7 +760,7 @@ private HasMetadata convertKubernetesItemToOpenShift(HasMetadata item) {
}

KubernetesToOpenShiftConverter converter = openShiftConverters.get(item.getKind());
return converter != null ? converter.convert(item, trimImageInContainerSpec) : item;
return converter != null ? converter.convert(item, trimImageInContainerSpec, setImageTriggerAutomatic) : item;
}

// ==================================================================================
Expand Down

3 comments on commit 8203161

@mojsha
Copy link

@mojsha mojsha commented on 8203161 Feb 13, 2018

Choose a reason for hiding this comment

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

@rohanKanojia
That looks perfect!

One final thing that I forgot: There is an automatic ConfigChangeTrigger also a few lines before your change to the ImageTrigger:

            // lets add a default trigger so that its triggered when we change its config
            specBuilder.addNewTrigger().withType("ConfigChange").endTrigger();

This is also causing us issues in Production where we don't want to have automatic re-deployment just because we change the DC. Usuallly in enterprises you need a manual approval in Production before something goes live and this breaks that.

Can you either remove this ConfigChangeTrigger, put it under the same flag or alternatively make another flag for it? Just a way to make it optional basically.

@mojsha
Copy link

@mojsha mojsha commented on 8203161 Feb 20, 2018

Choose a reason for hiding this comment

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

@rohanKanojia Ha ve you had time to look into this?

@rohanKanojia
Copy link
Owner Author

Choose a reason for hiding this comment

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

@mojsha : Please have a look at #1182.

Please sign in to comment.