Add rfc for script run stage #4603

ffjlabo · 2023-10-05T06:23:44Z

What this PR does / why we need it:

Which issue(s) this PR fixes:

Part of #4643

Does this PR introduce a user-facing change?:

How are users affected by this change:
Is this breaking change:
How to migrate (if breaking change):

codecov · 2023-10-05T06:29:28Z

Codecov Report

All modified and coverable lines are covered by tests ✅

❗ No coverage uploaded for pull request base (master@7366b31). Click here to learn what that means.

Additional details and impacted files

@@            Coverage Diff            @@
##             master    #4603   +/-   ##
=========================================
  Coverage          ?   29.99%           
=========================================
  Files             ?      221           
  Lines             ?    25937           
  Branches          ?        0           
=========================================
  Hits              ?     7781           
  Misses            ?    17510           
  Partials          ?      646

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Signed-off-by: Yoshiki Fujikane <[email protected]>

kentakozuka · 2023-10-24T00:32:54Z

@ffjlabo Will the custom sync be removed after this feature is released?
It seems like this script run stage has all the features that the custom sync has.

ffjlabo · 2023-10-24T03:20:48Z

@kentakozuka Yes, I think it will be deprecated and removed in the future.
Custom sync is a great way to execute any command. I think it is more useful to execute any command with other stages. But now we can't use custom sync with other stages such as K8S_SYNC because it is a kind of application.
So I proposed the script run stage to execute any command with other stages.

kentakozuka · 2023-10-24T05:31:28Z

@ffjlabo 👌

But now we can't use custom sync with other stages such as K8S_SYNC because it is a kind of application.

What do you mean by this? Maybe I miss something 🤔

ffjlabo · 2023-10-24T05:56:31Z

@kentakozuka

But now we can't use custom sync with other stages such as K8S_SYNC because it is a kind of application.

Oh, I'm sorry. I don't understand about custom sync correctly. custom sync can be used at any other stage. 🙏
https://pipecd.dev/docs-v0.45.x/user-guide/managing-application/customizing-deployment/custom-sync/

But this stage is for sync. In custom sync, the defined command will execute the same command when rollback.

By the script run stage, I want to realize the demand to just execute commands before or after other stages.
If we want to execute when rollback, we can define any command.

kentakozuka · 2023-10-24T05:59:22Z

@ffjlabo Thanks! It is clear to me now.
Can you write that the custom sync would be deprecated when the script run stage is implemented?

Signed-off-by: Yoshiki Fujikane <[email protected]>

ffjlabo · 2023-10-24T09:47:10Z

@kentakozuka Thank you for your advise! I added it 🙏 5f74cfd

khanhtc1202 · 2023-10-25T02:47:35Z

docs/rfcs/0011-script-run-stage.md

+- It might be better to add `runsOnRollback` on each sync stage if users want to control when rollback.
+This is a just idea.


I don't get this part, do you mean in case of multiple script_run stages are combined, we should add runsOnRollback to each? And is this runsOnRollback is same as onRollback (the one used in the example above)?

@khanhtc1202
This means It might be a way to add onRollback to each stage, not to create a new stage.

e.g. K8S_CANARY_ROLLOUT

pipeline: stages: - name: K8S_CANARY_ROLLOUT with: replicas: 10% onRollback: "curl -X POST -H 'Content-type: application/json' --data '{"text":"failed to deploy: rollback"}' $SLACK_WEBHOOK_URL"

Why does it come to this place 🤔 And also, how do we combine your idea with the current rollback logic? Do we have any plans for this?

Why does it come to this place 🤔 And also, how do we combine your idea with the current rollback logic? Do we have any plans for this?

It is a just idea, so I have no idea to realize it 🙇
But I thought it might be more useful to be able to set onRollback on each stages than set a stage to execute any command.

khanhtc1202 · 2023-10-25T02:51:11Z

docs/rfcs/0011-script-run-stage.md

+          onRollback:
+            - "curl -X POST -H 'Content-type: application/json' --data '{"text":"failed to deploy: rollback"}' $SLACK_WEBHOOK_URL"
+```
+


Some other questions I think we should define before starting:

Do we support multi script_run stages combine

How the user-defined onRollback be run in case of multi script_run stages combine

How the user-defined onRollback be run in combination with the XXX_ROLLBACK stage? I mean, if k8s app is rollbacked and the pipeline contains script_run, how do we perform the rollback process? Should it be XXX_ROLLBACK first then SCRIPT_RUN_ROLLBACK?

Thank you for your questions!
I will write about these things on rfc, but in advance, I will write a little bit my opinions on this.

Do we support multi script_run stages combine

Yes, I think so.

How the user-defined onRollback be run in case of multi script_run stages combine

Multi script_run stages can be executed in the order these are set.

How the user-defined onRollback be run in combination with the XXX_ROLLBACK stage? I mean, if k8s app is rollbacked and the pipeline contains script_run, how do we perform the rollback process? Should it be XXX_ROLLBACK first then SCRIPT_RUN_ROLLBACK?

I don't think about it. I will try to check the implementation and propose it 🙇

Multi script_run stages can be executed in the order these are set.

How about those stages' rollback scripts? Which order should be made, which will we choice? (or neither)

xxx_canary -> script_run -> script_run -> rollback -> script_rollback (merge all script rollback)

xxx_canary -> script_run -> script_run -> rollback -> script_rollback -> script_rollback

I think it will be 2. (But I don't know yet it can be realized technically 🙇 )
xxx_canary -> script_run -> script_run -> rollback -> script_rollback -> script_rollback

I think it should be better not to merge rollback scripts because it would be easy to handle or log error when to execute each rollback scripts.

ffjlabo · 2023-12-08T05:51:14Z

📝 How to prepare tools that we want to use

There are several ways to realize so.

install the tools on the piped container. It might be nice to prepare piped image based on ubuntu
make a custom piped container image in which some tools are installed.

khanhtc1202 · 2023-12-08T05:56:41Z

@ffjlabo Please refer to this discussion, we noted about this before
#4380

ffjlabo · 2023-12-22T05:53:13Z

@khanhtc1202
I implemented the stage as an alpha version. So, I want to merge this PR and work on the rollback implementation. 🙏
#4720

I will describe about the rollback on the original issue and fix the RFC on another PR :)

t-kikuc

Great

khanhtc1202

Here you go 🚀

* Add rfc for script run stage Signed-off-by: Yoshiki Fujikane <[email protected]> * Fix docs Signed-off-by: Yoshiki Fujikane <[email protected]> * Fix document Signed-off-by: Yoshiki Fujikane <[email protected]> --------- Signed-off-by: Yoshiki Fujikane <[email protected]>

Add rfc for script run stage

7476dd2

Signed-off-by: Yoshiki Fujikane <[email protected]>

ffjlabo force-pushed the add-rfc-script-run-stage branch from c03f147 to 7476dd2 Compare October 5, 2023 06:40

Fix docs

e705908

Signed-off-by: Yoshiki Fujikane <[email protected]>

ffjlabo marked this pull request as ready for review October 19, 2023 07:19

ffjlabo requested review from nghialv, khanhtc1202 and kentakozuka as code owners October 19, 2023 07:19

Fix document

5f74cfd

Signed-off-by: Yoshiki Fujikane <[email protected]>

kentakozuka mentioned this pull request Oct 24, 2023

Dynamic mentions to slack #4417

Closed

khanhtc1202 reviewed Oct 25, 2023

View reviewed changes

ffjlabo mentioned this pull request Dec 18, 2023

Introduce script run stage #4643

Open

7 tasks

ffjlabo requested a review from khanhtc1202 December 22, 2023 05:53

t-kikuc self-requested a review December 26, 2023 07:54

t-kikuc approved these changes Dec 26, 2023

View reviewed changes

khanhtc1202 approved these changes Dec 26, 2023

View reviewed changes

khanhtc1202 merged commit d814680 into master Dec 26, 2023
14 checks passed

khanhtc1202 deleted the add-rfc-script-run-stage branch December 26, 2023 07:56

github-actions bot mentioned this pull request Feb 6, 2024

Release v0.46.0 #4775

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add rfc for script run stage #4603

Add rfc for script run stage #4603

ffjlabo commented Oct 5, 2023 •

edited

Loading

codecov bot commented Oct 5, 2023 •

edited

Loading

kentakozuka commented Oct 24, 2023

ffjlabo commented Oct 24, 2023

kentakozuka commented Oct 24, 2023

ffjlabo commented Oct 24, 2023

kentakozuka commented Oct 24, 2023

ffjlabo commented Oct 24, 2023

khanhtc1202 Oct 25, 2023

ffjlabo Oct 25, 2023

khanhtc1202 Oct 26, 2023

ffjlabo Oct 26, 2023

khanhtc1202 Oct 25, 2023

ffjlabo Oct 25, 2023 •

edited

Loading

khanhtc1202 Oct 26, 2023 •

edited

Loading

ffjlabo Oct 26, 2023

ffjlabo Oct 26, 2023

ffjlabo commented Dec 8, 2023

khanhtc1202 commented Dec 8, 2023

ffjlabo commented Dec 22, 2023

t-kikuc left a comment

khanhtc1202 left a comment

		- It might be better to add `runsOnRollback` on each sync stage if users want to control when rollback.
		This is a just idea.

Add rfc for script run stage #4603

Add rfc for script run stage #4603

Conversation

ffjlabo commented Oct 5, 2023 • edited Loading

codecov bot commented Oct 5, 2023 • edited Loading

Codecov Report

kentakozuka commented Oct 24, 2023

ffjlabo commented Oct 24, 2023

kentakozuka commented Oct 24, 2023

ffjlabo commented Oct 24, 2023

kentakozuka commented Oct 24, 2023

ffjlabo commented Oct 24, 2023

khanhtc1202 Oct 25, 2023

Choose a reason for hiding this comment

ffjlabo Oct 25, 2023

Choose a reason for hiding this comment

khanhtc1202 Oct 26, 2023

Choose a reason for hiding this comment

ffjlabo Oct 26, 2023

Choose a reason for hiding this comment

khanhtc1202 Oct 25, 2023

Choose a reason for hiding this comment

ffjlabo Oct 25, 2023 • edited Loading

Choose a reason for hiding this comment

khanhtc1202 Oct 26, 2023 • edited Loading

Choose a reason for hiding this comment

ffjlabo Oct 26, 2023

Choose a reason for hiding this comment

ffjlabo Oct 26, 2023

Choose a reason for hiding this comment

ffjlabo commented Dec 8, 2023

khanhtc1202 commented Dec 8, 2023

ffjlabo commented Dec 22, 2023

t-kikuc left a comment

Choose a reason for hiding this comment

khanhtc1202 left a comment

Choose a reason for hiding this comment

ffjlabo commented Oct 5, 2023 •

edited

Loading

codecov bot commented Oct 5, 2023 •

edited

Loading

ffjlabo Oct 25, 2023 •

edited

Loading

khanhtc1202 Oct 26, 2023 •

edited

Loading