feat(pipeline_templates): Display "Force Rebake" checkbox for MPT #5854

jervi · 2018-10-16T12:13:37Z

When manually executing templated pipelines, the "Force Rebake" checkbox is missing, even though the pipeline contains a bake stage. Templated pipelines do not contain any stages before runtime, so the bake stage is never asked to inject its custom logic into the manual execution modal.

This is a simple 90/10-solution that just ~~iterates through previous executions, finds the previous execution that actually ran, and naïvely copies the~~ plans a new execution and adds the stages to the pipeline, so that the bake stage magic is executed.
It might result in false positives or negatives if you are changing the pipeline template, but as the main structure of a pipeline rarely changes and the fact that there are no bad consequences of displaying a "Force Rebake" checkbox when there is no bake stage, I think this is good enough.

jervi · 2018-10-23T11:00:53Z

@spinnaker/reviewers PTAL

christopherthielen · 2018-10-24T22:47:46Z

I'm not sure I'm comfortable with this, but I don't know MPT well enough to offer a better suggestion

robzienert · 2018-10-24T23:37:19Z

It's awkward for sure, but is the pattern we've gone with on the backend as well for outlier behaviors in MPT.

danielpeach · 2018-10-24T23:45:32Z

app/scripts/modules/core/src/pipeline/manualExecution/manualPipelineExecution.controller.js

+      ) {
+        const previousSuccessfulExecution = executions.find(e => e.stages.length > 0);
+        if (previousSuccessfulExecution) {
+          pipeline.stages = previousSuccessfulExecution.stages;


Could you plan the pipeline here instead? Just a thought.

jervi · 2018-10-25T21:28:21Z

app/scripts/modules/core/src/pipeline/manualExecution/manualPipelineExecution.controller.js

@@ -109,6 +110,21 @@ module.exports = angular
      this.command.trigger = suppliedTriggerCanBeInvoked ? trigger : _.head(this.triggers);
    };

+    const updatePipelinePlan = pipeline => {
+      if (pipeline.type === 'templatedPipeline' && (pipeline.stages === undefined || pipeline.stages.length === 0)) {
+        PipelineTemplateReader.getPipelinePlan(pipeline)


getPipelinePlan takes an optional second parameter executionId. Ideally I should've used the build selected in the build drop down if available, but that turned out to be a rabbit hole. The builds in that list only have the CI build number, not the executionId. The executions variable only have a few executions for each pipeline (depending on the number of pipelines being displayed). And since the dropdown is a react component, I have to use a $watch expression to detect when it changes.
So the current implementation will only consider the newest version of the pipeline when deciding wether to show the force rebake box or not. But I think this is Good Enough™, for the same reasons mentioned earlier. It's certainly better than never showing it.

jervi · 2018-10-25T21:33:15Z

Take 2! It's not perfect, but it's better. I have implemented the plan suggestion from @danielpeach, which is of course the way it should have been done. The implementation is still a bit naïve, but I don't think it's worth it to put a lot more effort into this.

christopherthielen

nice!

When manually executing templated pipelines, the "Force Rebake" checkbox is missing, even though the pipeline contains a bake stage. Templated pipelines do not contain any stages before runtime, so the bake stage is never asked to inject its custom logic into the manual execution modal. This is a simple 90/10-solution that just iterates through previous executions, finds the previous execution that actually ran, and naïvely copies the stages to the pipeline, so that the bake stage magic is executed. It might result in false positives and negatives if you are changing the pipeline template, but as the main structure of a pipeline rarely changes and the fact that there are no bad consequences of displaying a force rebake checkbox when there is no bake stage, I think this is good enough.

57b1e49 chore(core/presentation): Update formik from 0.11.11 to 1.3.1 (spinnaker#5917) c0842ad fix(provider/cf): fetch services on a per-region basis f22fecd feat(core/presentation): Introduce Validation class for reusable validation fns. (spinnaker#5913) 9eacd0c chore(core/pipeline): Cleaning up imports (spinnaker#5914) b08bab2 refactor(core/projects): Simplify TrashButton click handler 1abc6ba refactor(core): Remove invalid <g> from HoverablePopover (spinnaker#5904) 8523bbd test(core/cluster): fix lint error unused variable (spinnaker#5910) cc68c9c feat(core): Support a date constraint on trigger parameters (spinnaker#5907) 9488e33 feat(pipeline_templates): Display "Force Rebake" checkbox for MPT (spinnaker#5854)

57b1e49 chore(core/presentation): Update formik from 0.11.11 to 1.3.1 (#5917) c0842ad fix(provider/cf): fetch services on a per-region basis f22fecd feat(core/presentation): Introduce Validation class for reusable validation fns. (#5913) 9eacd0c chore(core/pipeline): Cleaning up imports (#5914) b08bab2 refactor(core/projects): Simplify TrashButton click handler 1abc6ba refactor(core): Remove invalid <g> from HoverablePopover (#5904) 8523bbd test(core/cluster): fix lint error unused variable (#5910) cc68c9c feat(core): Support a date constraint on trigger parameters (#5907) 9488e33 feat(pipeline_templates): Display "Force Rebake" checkbox for MPT (#5854)

…innaker#5854) * feat(pipeline_templates): Display "Force Rebake" checkbox for MPT When manually executing templated pipelines, the "Force Rebake" checkbox is missing, even though the pipeline contains a bake stage. Templated pipelines do not contain any stages before runtime, so the bake stage is never asked to inject its custom logic into the manual execution modal. This is a simple 90/10-solution that plans a new execution and adds the stages to the pipeline, so that the bake stage magic is executed. It might result in false positives or negatives if you are changing the pipeline template, but as the main structure of a pipeline rarely changes and the fact that there are no bad consequences of displaying a "Force Rebake" checkbox when there is no bake stage, I think this is good enough.

57b1e49 chore(core/presentation): Update formik from 0.11.11 to 1.3.1 (spinnaker#5917) c0842ad fix(provider/cf): fetch services on a per-region basis f22fecd feat(core/presentation): Introduce Validation class for reusable validation fns. (spinnaker#5913) 9eacd0c chore(core/pipeline): Cleaning up imports (spinnaker#5914) b08bab2 refactor(core/projects): Simplify TrashButton click handler 1abc6ba refactor(core): Remove invalid <g> from HoverablePopover (spinnaker#5904) 8523bbd test(core/cluster): fix lint error unused variable (spinnaker#5910) cc68c9c feat(core): Support a date constraint on trigger parameters (spinnaker#5907) 9488e33 feat(pipeline_templates): Display "Force Rebake" checkbox for MPT (spinnaker#5854)

This fix was first introduced in spinnaker#5854, but went MIA when the manual execution modal was converted to React.

#7558) This fix was first introduced in #5854, but went MIA when the manual execution modal was converted to React.

spinnaker#7558) This fix was first introduced in spinnaker#5854, but went MIA when the manual execution modal was converted to React.

jervi force-pushed the force_rebake_for_templated_pipelines branch 2 times, most recently from 18361b1 to 01b7af0 Compare October 23, 2018 11:13

danielpeach reviewed Oct 24, 2018

View reviewed changes

jervi force-pushed the force_rebake_for_templated_pipelines branch 2 times, most recently from 075dcb0 to 35b528c Compare October 25, 2018 21:18

jervi commented Oct 25, 2018

View reviewed changes

christopherthielen self-requested a review October 25, 2018 21:39

christopherthielen approved these changes Oct 25, 2018

View reviewed changes

jervi added 2 commits October 29, 2018 09:49

Take 2: Planning a new execution instead of using a previous one

27060e9

jervi force-pushed the force_rebake_for_templated_pipelines branch from 35b528c to 27060e9 Compare October 29, 2018 08:50

gardleopard merged commit 9488e33 into spinnaker:master Oct 29, 2018

spinnakerbot added the target-release/1.11 label Oct 29, 2018

jervi deleted the force_rebake_for_templated_pipelines branch October 29, 2018 20:06

christopherthielen mentioned this pull request Oct 31, 2018

chore(core): Bump version to 0.0.281 #5919

Merged

jervi added a commit to jervi/deck that referenced this pull request Oct 24, 2019

fix(core/pipeline): Force rebake not displayed for templated pipelines

d15b296

This fix was first introduced in spinnaker#5854, but went MIA when the manual execution modal was converted to React.

jervi mentioned this pull request Oct 24, 2019

fix(core/pipeline): Force rebake not displayed for templated pipelines #7558

Merged

christopherthielen pushed a commit that referenced this pull request Oct 24, 2019

fix(core/pipeline): Force rebake not displayed for templated pipelines (

17bf82a

#7558) This fix was first introduced in #5854, but went MIA when the manual execution modal was converted to React.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(pipeline_templates): Display "Force Rebake" checkbox for MPT #5854

feat(pipeline_templates): Display "Force Rebake" checkbox for MPT #5854

jervi commented Oct 16, 2018 •

edited

Loading

jervi commented Oct 23, 2018

christopherthielen commented Oct 24, 2018

robzienert commented Oct 24, 2018

danielpeach Oct 24, 2018

jtk54 Oct 25, 2018

jervi Oct 25, 2018 •

edited

Loading

jervi commented Oct 25, 2018

christopherthielen left a comment

feat(pipeline_templates): Display "Force Rebake" checkbox for MPT #5854

feat(pipeline_templates): Display "Force Rebake" checkbox for MPT #5854

Conversation

jervi commented Oct 16, 2018 • edited Loading

jervi commented Oct 23, 2018

christopherthielen commented Oct 24, 2018

robzienert commented Oct 24, 2018

danielpeach Oct 24, 2018

Choose a reason for hiding this comment

jtk54 Oct 25, 2018

Choose a reason for hiding this comment

jervi Oct 25, 2018 • edited Loading

Choose a reason for hiding this comment

jervi commented Oct 25, 2018

christopherthielen left a comment

Choose a reason for hiding this comment

jervi commented Oct 16, 2018 •

edited

Loading

jervi Oct 25, 2018 •

edited

Loading