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

Minor changes to ImageTrigger generation #1177

Closed
mojsha opened this issue Feb 8, 2018 · 9 comments
Closed

Minor changes to ImageTrigger generation #1177

mojsha opened this issue Feb 8, 2018 · 9 comments
Assignees
Labels
prio/p0 Highest priority
Milestone

Comments

@mojsha
Copy link

mojsha commented Feb 8, 2018

We currently have a few small requirements which has led us to do our own hacks for the F-M-P and this causes issues whenever there is an upgrade to F-M-P.

  1. We want to use the ImageTrigger to be able to resolve the ImageStreamTag but do not want automated deployments whenever there is a new Image generated. The only way to do so is to have a "disabled" ImageTrigger, meaning automatic is set to false.

  2. We also need to be able to specify the namespace for the ImageTrigger, so that we can refer to images in other namespaces. In our case, we have some images that are of common concern and we want to use this across all of our applications.

This is how it would look like:

    - imageChangeParams:
        automatic: false
        containerNames:
          - demoapp
        from:
          kind: ImageStreamTag
          name: 'common-concern-image:1.0.0'
          namespace: public-images

I have changed the following code in DeploymentOpenShiftConverter.java:

                if (containerToImageMap.size() != 0) {
                    if (mode.equals(PlatformMode.openshift)) {
                        for (Map.Entry<String, String> entry : containerToImageMap.entrySet()) {
                            String containerName = entry.getKey();
                            ImageName image = new ImageName(entry.getValue());
                            String tag = image.getTag() != null ? image.getTag() : "latest";
                            specBuilder.addNewTrigger()
                                    .withType("ImageChange")
                                    .withNewImageChangeParams()
                                    .withAutomatic(false) // This will set automatic to false
                                    .withNewFrom()
                                    .withKind("ImageStreamTag")
                                    .withName(image.getSimpleName() + ":" + tag)
                                    .withNamespace(image.getUser()) // This will get the namespace
                                    .endFrom()
                                    .withContainerNames(containerName)
                                    .endImageChangeParams()
                                    .endTrigger();
                        }
                    }
                }

Can these be set as options?

@rohanKanojia

@rohanKanojia
Copy link
Member

rohanKanojia commented Feb 8, 2018

We can add a flag which you can use in your build like this:

   mvn clean install -Dfabric8.openshift.isImageTriggerAutomatic=false fabric8:deploy

It's default value would be true(which is the behavior of fabric8 maven plugin right now). How does that sound to you?
We can add a separate flag for namespace also.

@hrishin Could you please share your thoughts on this?

@mojsha
Copy link
Author

mojsha commented Feb 8, 2018

Sounds great. Though, could we rename it to setImageTriggerAutomatic (or something like that) instead of isImageTriggerAutomatic? Because it's a bit confusing when using isImageTriggerAutomatic.

Will the namespace be added by default without a flag? I'm not sure if it causes any harm to the current usecases.

@rohanKanojia
Copy link
Member

It depends on whether it's specific to your use case only. But I feel that they should be given as two separate flags. I need to check if it causes any harm to default flow of fmp.

@hrishin
Copy link
Member

hrishin commented Feb 9, 2018

@rohanKanojia @mojsha No issues right now. Yes keep default value to true

@mojsha
Copy link
Author

mojsha commented Feb 12, 2018

@rohanKanojia Any option is fine. I just need this ASAP to avoid doing my home-made "patches" on top of the releases from F-M-P. Much appreciated.

@rohanKanojia rohanKanojia added prio/p0 Highest priority WIP labels Feb 12, 2018
rohanKanojia added a commit to rohanKanojia/fabric8-maven-plugin that referenced this issue Feb 12, 2018
Added fabric8.openshift.setImageTriggerAutomatic flag which would
be able to enable/disable automatic deployments.
rohanKanojia added a commit to rohanKanojia/fabric8-maven-plugin that referenced this issue Feb 13, 2018
Added fabric8.openshift.setImageTriggerAutomatic flag which would
be able to enable/disable automatic deployments.
rohanKanojia added a commit to rohanKanojia/fabric8-maven-plugin that referenced this issue Feb 20, 2018
Added fabric8.openshift.setDeploymentTriggerAutomatic flag which would
be able to enable/disable automatic deployments.
@mojsha
Copy link
Author

mojsha commented Feb 20, 2018

@rohanKanojia Looks perfect!

@piyush-garg piyush-garg added this to the Sprint 145 H milestone Feb 20, 2018
rohanKanojia added a commit to rohanKanojia/fabric8-maven-plugin that referenced this issue Mar 1, 2018
Added fabric8.openshift.setDeploymentTriggerAutomatic flag which would
be able to enable/disable automatic deployments.
rohanKanojia added a commit to rohanKanojia/fabric8-maven-plugin that referenced this issue Mar 2, 2018
Added fabric8.openshift.setDeploymentTriggerAutomatic flag which would
be able to enable/disable automatic deployments.
rohanKanojia added a commit to rohanKanojia/fabric8-maven-plugin that referenced this issue Mar 6, 2018
Added fabric8.openshift.setDeploymentTriggerAutomatic flag which would
be able to enable/disable automatic deployments.
@mojsha
Copy link
Author

mojsha commented Mar 6, 2018

@rohanKanojia Thanks alot for your efforts! This will help us with the 3.7 upgrade/migration. Can I ask how long it will take for this to be released?

rohanKanojia added a commit to rohanKanojia/fabric8-maven-plugin that referenced this issue Mar 6, 2018
Added fabric8.openshift.setDeploymentTriggerAutomatic flag which would
be able to enable/disable automatic deployments.
@piyush-garg
Copy link
Collaborator

Hey @mojsha

We just released FMP 3.5.38. Please give it a try. Looking forward to your feedback.

Thanks

@mojsha
Copy link
Author

mojsha commented Mar 12, 2018

@piyush1594 @rohanKanojia I have tested the plugin and tentatively it looks like it's working as it should. I would like to thank you and in particular @rohanKanojia for all of his efforts! This has enabled us to start our journey towards OpenShift 3.7+.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
prio/p0 Highest priority
Projects
None yet
Development

No branches or pull requests

4 participants