[docs] Explain the builtin commands and the separating script for script run stage #5127

ffjlabo · 2024-08-09T11:16:49Z

What this PR does / why we need it:

as title

Which issue(s) this PR fixes:

Fixes #

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):

… stage Signed-off-by: Yoshiki Fujikane <ffjlabo@gmail.com>

codecov · 2024-08-09T11:22:57Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 22.84%. Comparing base (58bf4ff) to head (48022da).
Report is 50 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #5127      +/-   ##
==========================================
+ Coverage   22.80%   22.84%   +0.04%     
==========================================
  Files         412      420       +8     
  Lines       43827    45247    +1420     
==========================================
+ Hits         9996    10338     +342     
- Misses      33044    34114    +1070     
- Partials      787      795       +8

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

khanhtc1202 · 2024-08-09T11:32:45Z

docs/content/en/docs-dev/user-guide/managing-application/customizing-deployment/script-run.md

+- ssh
+- jq
+- curl
+- and the builtin commands installed in the base image.


Users can also use piped installed tools such as terraform, kubectl, helm, kustomize, etc 👀

@khanhtc1202
Thank you for the comment.
I think it might be unexpected usage for these tools.
WDYT?

Sorry, I don't really get your point. Could you explain it 🙏

📝 Discussed about it in person. I agree that users can use the commands installed for the piped.

we can use them on the script run stage

but this may change in the future

fixed on 48d7819

I agree with @khanhtc1202 .
It's okay to mention that some tools may be available under PIPED_TOOLS_DIR.
But I think we also have to mention that the tools can be absent sometimes.

@khanhtc1202 @Warashi

So, the binaries are sometimes absent, such as when the SCRIPT_RUN stage is the first stage of the pipeline and the binaries are not installed yet.

I forgot it. So I think we should not explain it.

It might be confusing for users who use SCRIPT_RUN that the command does not exist until the first deployment is executed.
If the piped administrator and the app administrator are different, I don't think there is a way for the app administrator to check the dir.

WDYT?

The app administrator can use the SCRIPT_RUN stage, such as executing ls, to check whether the commands exist.
But I think it's nonsense to check whether the commands exist. If the administrator has to use a command, they should install it in the SCRIPT_RUN stage or ask the piped admin to use a custom container image containing the command.

I think it's okay to mention some tools are sometimes available under PIPED_TOOLS_DIR because it's a fact.
On the other hand, users must use those tools carefully, and we must explain why users must pay attention.

I tried adding it anyway. bfb1765

Plz check it 🙏 @Warashi @khanhtc1202

t-kikuc

Great improvement 👍
I left some nits

t-kikuc · 2024-08-10T13:37:44Z

docs/content/en/docs-dev/user-guide/managing-application/customizing-deployment/script-run.md

+  labels:
+    env: example
+    team: product


[nits] I think labels section is not necessary for the explanation.

t-kikuc · 2024-08-10T13:37:46Z

docs/content/en/docs-dev/user-guide/managing-application/customizing-deployment/script-run.md

+
+The public piped image available in PipeCD main repo (ref: [Dockerfile](https://github.com/pipe-cd/pipecd/blob/master/cmd/piped/Dockerfile)) is based on [alpine](https://hub.docker.com/_/alpine/) and only has a few UNIX command available (ref: [piped-base Dockerfile](https://github.com/pipe-cd/pipecd/blob/master/tool/piped-base/Dockerfile)). 
+
+If you want to use your commands, you can:


[nits] I wanna clarify whether the options are isolated or combined.

e.g.

Suggested change

If you want to use your commands, you can:

If you want to use your commands, follow these steps:

t-kikuc · 2024-08-10T13:39:52Z

docs/content/en/docs-dev/user-guide/managing-application/customizing-deployment/script-run.md

+└── script.sh
+```
+
+## Builtin command


[nits]

Suggested change

## Builtin command

## Builtin commands

t-kikuc · 2024-08-10T13:40:54Z

docs/content/en/docs-dev/user-guide/managing-application/customizing-deployment/script-run.md

+- curl
+- and the builtin commands installed in the base image.
+
+The public piped image available in PipeCD main repo (ref: [Dockerfile](https://github.com/pipe-cd/pipecd/blob/master/cmd/piped/Dockerfile)) is based on [alpine](https://hub.docker.com/_/alpine/) and only has a few UNIX command available (ref: [piped-base Dockerfile](https://github.com/pipe-cd/pipecd/blob/master/tool/piped-base/Dockerfile)). 


[nits]

Suggested change

The public piped image available in PipeCD main repo (ref: [Dockerfile](https://github.com/pipe-cd/pipecd/blob/master/cmd/piped/Dockerfile)) is based on [alpine](https://hub.docker.com/_/alpine/) and only has a few UNIX command available (ref: [piped-base Dockerfile](https://github.com/pipe-cd/pipecd/blob/master/tool/piped-base/Dockerfile)).

The public piped image available in PipeCD main repo (ref: [Dockerfile](https://github.com/pipe-cd/pipecd/blob/master/cmd/piped/Dockerfile)) is based on [alpine](https://hub.docker.com/_/alpine/) and only has a few UNIX commands available (ref: [piped-base Dockerfile](https://github.com/pipe-cd/pipecd/blob/master/tool/piped-base/Dockerfile)).

Signed-off-by: Yoshiki Fujikane <ffjlabo@gmail.com>

ffjlabo · 2024-08-13T02:13:42Z

@t-kikuc Thank you for the comment 🙏 I fixed them.

fix description for the usecase 3ab416e
- clarified the options as isolated ones.
- removed the one that is not recommended
remove labels ed4f0b0
wrong spell c502a27

Warashi · 2024-08-19T04:56:23Z

...content/en/docs-v0.46.x/user-guide/managing-application/customizing-deployment/script-run.md

+For example, If you are using the container platform and the offcial piped container image, you can use the command below.
+- git
+- ssh
+- jq


We don't install jq in 0.46.x images, because it's installed at 0.48.0 release
#5009

fixed on dc78b67

Warashi · 2024-08-19T04:57:42Z

...content/en/docs-v0.47.x/user-guide/managing-application/customizing-deployment/script-run.md

+For example, If you are using the container platform and the offcial piped container image, you can use the command below.
+- git
+- ssh
+- jq


We don't install jq in 0.47.x images, because it's installed at 0.48.0 release
#5009

fixed on dc78b67

khanhtc1202 · 2024-08-30T04:05:27Z

@ffjlabo Could you check this 👀

Signed-off-by: Yoshiki Fujikane <ffjlabo@gmail.com>

Warashi

LGTM

t-kikuc · 2024-09-05T01:50:35Z

@khanhtc1202 Would you check again?

khanhtc1202 · 2024-09-05T02:58:17Z

docs/content/en/docs-dev/user-guide/managing-application/customizing-deployment/script-run.md

+- git
+- ssh
+- jq
+- curl
+- kubectl, helm, kustomize, terraform stored in $PIPED_TOOL_DIR 
+  - Be careful. These tools are sometimes absent because they are installed when the first deployment has been done using each tool. Please check $PIPED_TOOL_DIR before you use them.
+  - Binaries can be specified as "kubectl-${version}" or "kubectl". 
+  - If no version is specified on the piped config or app.pipecd.yaml, both the ones with and without default version suffix will be installed. 
+  - Please check the [code](https://github.com/pipe-cd/pipecd/blob/master/pkg/app/piped/toolregistry/install.go#L28C1-L33C2) about the default versions.
+- and the builtin commands installed in the base image.


Suggested change

- git

- ssh

- jq

- curl

- kubectl, helm, kustomize, terraform stored in $PIPED_TOOL_DIR

- Be careful. These tools are sometimes absent because they are installed when the first deployment has been done using each tool. Please check $PIPED_TOOL_DIR before you use them.

- Binaries can be specified as "kubectl-${version}" or "kubectl".

- If no version is specified on the piped config or app.pipecd.yaml, both the ones with and without default version suffix will be installed.

- Please check the [code](https://github.com/pipe-cd/pipecd/blob/master/pkg/app/piped/toolregistry/install.go#L28C1-L33C2) about the default versions.

- and the builtin commands installed in the base image.

- git

- ssh

- jq

- curl

- commands installed by piped in $PIPED_TOOL_DIR (check at runtime)

- built-in commands installed in the base image

How about this? @ffjlabo

@khanhtc1202
I think we also have to mention that the tools can be absent sometimes.
WDYT?

That is why I added the note (check at runtime). I think it would be better not to confuse users by showing too much explanation 🤔

OK, I agree with your point.
I will fix it👌

fixed 48022da

Signed-off-by: Yoshiki Fujikane <ffjlabo@gmail.com>

khanhtc1202

Thank you 💯

Explain the builtin commands and the separating script for script run…

5f58c28

… stage Signed-off-by: Yoshiki Fujikane <ffjlabo@gmail.com>

ffjlabo requested review from nghialv, khanhtc1202, kentakozuka, t-kikuc and Warashi as code owners August 9, 2024 11:16

khanhtc1202 reviewed Aug 9, 2024

View reviewed changes

t-kikuc reviewed Aug 10, 2024

View reviewed changes

ffjlabo added 3 commits August 13, 2024 11:00

Fix the description for the usecase when we want to use any commands

3ab416e

Signed-off-by: Yoshiki Fujikane <ffjlabo@gmail.com>

Remove labels

ed4f0b0

Signed-off-by: Yoshiki Fujikane <ffjlabo@gmail.com>

Fix spell

c502a27

Signed-off-by: Yoshiki Fujikane <ffjlabo@gmail.com>

ffjlabo force-pushed the fix-docs-for-script-run branch from 105b378 to c502a27 Compare August 13, 2024 02:09

ffjlabo requested a review from t-kikuc August 13, 2024 02:13

t-kikuc previously approved these changes Aug 13, 2024

View reviewed changes

Warashi reviewed Aug 19, 2024

View reviewed changes

Remove jq from v0.46.x, v0.47.x

dc78b67

Signed-off-by: Yoshiki Fujikane <ffjlabo@gmail.com>

ffjlabo dismissed t-kikuc’s stale review via dc78b67 September 3, 2024 03:16

Add description for built in tools

48d7819

Signed-off-by: Yoshiki Fujikane <ffjlabo@gmail.com>

ffjlabo requested review from Warashi and khanhtc1202 September 3, 2024 03:53

Fix docs

bfb1765

Signed-off-by: Yoshiki Fujikane <ffjlabo@gmail.com>

Warashi previously approved these changes Sep 4, 2024

View reviewed changes

khanhtc1202 reviewed Sep 5, 2024

View reviewed changes

Fix briefly

48022da

Signed-off-by: Yoshiki Fujikane <ffjlabo@gmail.com>

ffjlabo dismissed Warashi’s stale review via 48022da September 5, 2024 09:19

ffjlabo requested a review from khanhtc1202 September 5, 2024 09:21

ffjlabo requested review from Warashi and t-kikuc September 5, 2024 09:21

t-kikuc approved these changes Sep 5, 2024

View reviewed changes

khanhtc1202 approved these changes Sep 5, 2024

View reviewed changes

khanhtc1202 merged commit e7c40a0 into master Sep 5, 2024
17 checks passed

khanhtc1202 deleted the fix-docs-for-script-run branch September 5, 2024 15:26

github-actions bot mentioned this pull request Sep 10, 2024

Release v0.48.9 #5205

Merged

github-actions bot mentioned this pull request Sep 24, 2024

Update RELEASE to v0.49.0 #5221

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[docs] Explain the builtin commands and the separating script for script run stage #5127

[docs] Explain the builtin commands and the separating script for script run stage #5127

ffjlabo commented Aug 9, 2024

codecov bot commented Aug 9, 2024 •

edited

Loading

khanhtc1202 Aug 9, 2024

ffjlabo Aug 9, 2024

khanhtc1202 Aug 13, 2024

ffjlabo Aug 13, 2024

ffjlabo Sep 3, 2024

Warashi Sep 3, 2024

ffjlabo Sep 3, 2024

Warashi Sep 3, 2024

ffjlabo Sep 4, 2024

ffjlabo Sep 4, 2024

t-kikuc left a comment

t-kikuc Aug 10, 2024

t-kikuc Aug 10, 2024

t-kikuc Aug 10, 2024

t-kikuc Aug 10, 2024

ffjlabo commented Aug 13, 2024

Warashi Aug 19, 2024

ffjlabo Sep 3, 2024

Warashi Aug 19, 2024

ffjlabo Sep 3, 2024

khanhtc1202 commented Aug 30, 2024

Warashi left a comment

t-kikuc commented Sep 5, 2024

khanhtc1202 Sep 5, 2024 •

edited

Loading

ffjlabo Sep 5, 2024

khanhtc1202 Sep 5, 2024

ffjlabo Sep 5, 2024

ffjlabo Sep 5, 2024

khanhtc1202 left a comment


		The public piped image available in PipeCD main repo (ref: [Dockerfile](https://github.com/pipe-cd/pipecd/blob/master/cmd/piped/Dockerfile)) is based on [alpine](https://hub.docker.com/_/alpine/) and only has a few UNIX command available (ref: [piped-base Dockerfile](https://github.com/pipe-cd/pipecd/blob/master/tool/piped-base/Dockerfile)).

		If you want to use your commands, you can:

	If you want to use your commands, you can:
	If you want to use your commands, follow these steps:

[docs] Explain the builtin commands and the separating script for script run stage #5127

[docs] Explain the builtin commands and the separating script for script run stage #5127

Conversation

ffjlabo commented Aug 9, 2024

codecov bot commented Aug 9, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

t-kikuc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ffjlabo commented Aug 13, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

khanhtc1202 commented Aug 30, 2024

Warashi left a comment

Choose a reason for hiding this comment

t-kikuc commented Sep 5, 2024

khanhtc1202 Sep 5, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

khanhtc1202 left a comment

Choose a reason for hiding this comment

codecov bot commented Aug 9, 2024 •

edited

Loading

khanhtc1202 Sep 5, 2024 •

edited

Loading