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 12, 2018
1 parent 2c40eac commit dbd8a0d
Show file tree
Hide file tree
Showing 7 changed files with 22 additions and 7 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
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,14 +119,24 @@ public HasMetadata convert(HasMetadata item, boolean trimImageInContainerSpec) {
specBuilder.addNewTrigger()
.withType("ImageChange")
.withNewImageChangeParams()
.withAutomatic(true)
.withAutomatic(setImageTriggerAutomatic)
.withNewFrom()
.withKind("ImageStreamTag")
.withName(image.getSimpleName() + ":" + tag)
.endFrom()
.withContainerNames(containerName)
.endImageChangeParams()
.endTrigger();

if(!setImageTriggerAutomatic) {
specBuilder.editLastTrigger()
.editImageChangeParams()
.editFrom()
.withNamespace(image.getUser())

This comment has been minimized.

Copy link
@mojsha

mojsha Feb 12, 2018

Do we want it here? As they are two different things. I would like to always specify the namespace.

This comment has been minimized.

Copy link
@rohanKanojia

rohanKanojia Feb 12, 2018

Author Owner

But as far as I remember you wanted both of them via one option. I'm not sure whether it's possible to provide both things (automatic flag and namespace) via a single flag.

This comment has been minimized.

Copy link
@mojsha

mojsha Feb 12, 2018

My point was more that this conditional (if I understood it correctly) covers the case where the flag is set to false.

But my scenario was that I needed both, i.e. even when the flag is set to true.

My suggestion was to always set the namespace in either condition if it otherwise doesn't harm the workflow of FMP. Otherwise, it should just be two separate flags as you suggested.

This comment has been minimized.

Copy link
@rohanKanojia

rohanKanojia Feb 12, 2018

Author Owner

okay. Thanks for clearing my doubt.

.endFrom()
.endImageChangeParams()
.endTrigger();
}
}
}
if(trimImageInContainerSpec) {
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

0 comments on commit dbd8a0d

Please sign in to comment.