-
Notifications
You must be signed in to change notification settings - Fork 183
Add README for k8s multi cluster plugin #5854
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Signed-off-by: Yoshiki Fujikane <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #5854 +/- ##
==========================================
- Coverage 27.80% 27.79% -0.02%
==========================================
Files 510 510
Lines 54301 54329 +28
==========================================
Hits 15101 15101
- Misses 38054 38082 +28
Partials 1146 1146 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Signed-off-by: Yoshiki Fujikane <[email protected]>
50c6131
to
6d06f3f
Compare
Signed-off-by: Yoshiki Fujikane <[email protected]>
1ec1f4c
to
ea6d194
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO: Let's add links to each plugin README in somewhere (like root README? pipedv1 README? or another) because this file is hard to find.
|
||
**Prepare the PipeCD Control Plane** | ||
|
||
Please refer to [pipe-cd/pipecd/cmd/pipecd/README.md](../../../../../cmd/pipecd/README.md) to set up the Control Plane in your local environment. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we need to specify the oldest version which is compatible with pipedv1 or just specify latest
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add comment on b29e4bf.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ffjlabo
Precisely, v0.51.3 does not contain some commits of plugins since they're not cherry picked.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to specify the commit when we released v0.51.3 instead of release-v0.51.x branch
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Precisely, v0.51.3 does not contain some commits of plugins since they're not cherry picked.
We need to specify the commit when we released v0.51.3 instead of release-v0.51.x branch
Thank you.
So, how about describing it using the upstream of the master branch?
The whole fixes for plugins are stored in that branch, and this is the docs for running locally with make command.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ffjlabo
I agree!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed on c012e03
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mention 'Currently only QuickSync is supported' somewhere
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed on dab25fd
At this time, please select multiple DeployTargets. | ||
|
||
## Examples | ||
There are examples under `./example`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the examples should be moved to https://github.com/pipe-cd/examples in the future.
What about for now? Put in examples/ or the examples repo?
WDYT? @Warashi @khanhtc1202
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the examples should be moved to https://github.com/pipe-cd/examples in the future.
I agree with it. I think the examples for built-in plugins should be located in it.
What about for now? Put in examples/ or the examples repo?
I think we should put them in examples for now.
Currently, some errors will occur when there are app.pipecd.yaml files for both pipedv0 and pipedv1.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should put them in examples for now.
I agree! It's still like a draft.
I approve, and if others have a different opinion, I'll reconsider it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with putting them under the plugin implementation directory for now because the configuration format may change in the future, as written in the head of this README. Putting them here is better because we can reduce the change scope in future pull requests; they are both under the plugin implementation directory.
Signed-off-by: Yoshiki Fujikane <[email protected]>
Signed-off-by: Yoshiki Fujikane <[email protected]>
Signed-off-by: Yoshiki Fujikane <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!!!
Let's try!!!!!!!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍🏻
* Add README for pipedv1 Signed-off-by: Yoshiki Fujikane <[email protected]> * Add README for k8s multi cluster Signed-off-by: Yoshiki Fujikane <[email protected]> * Add Config Reference Signed-off-by: Yoshiki Fujikane <[email protected]> --------- Signed-off-by: Yoshiki Fujikane <[email protected]> Signed-off-by: Tushar240503 <[email protected]>
* Add README for pipedv1 Signed-off-by: Yoshiki Fujikane <[email protected]> * Add README for k8s multi cluster Signed-off-by: Yoshiki Fujikane <[email protected]> * Add Config Reference Signed-off-by: Yoshiki Fujikane <[email protected]> --------- Signed-off-by: Yoshiki Fujikane <[email protected]> Signed-off-by: Tushar240503 <[email protected]>
* Add README for pipedv1 Signed-off-by: Yoshiki Fujikane <[email protected]> * Add README for k8s multi cluster Signed-off-by: Yoshiki Fujikane <[email protected]> * Add Config Reference Signed-off-by: Yoshiki Fujikane <[email protected]> --------- Signed-off-by: Yoshiki Fujikane <[email protected]> Signed-off-by: Tushar240503 <[email protected]>
* Add README for pipedv1 Signed-off-by: Yoshiki Fujikane <[email protected]> * Add README for k8s multi cluster Signed-off-by: Yoshiki Fujikane <[email protected]> * Add Config Reference Signed-off-by: Yoshiki Fujikane <[email protected]> --------- Signed-off-by: Yoshiki Fujikane <[email protected]>
* Add README for pipedv1 Signed-off-by: Yoshiki Fujikane <[email protected]> * Add README for k8s multi cluster Signed-off-by: Yoshiki Fujikane <[email protected]> * Add Config Reference Signed-off-by: Yoshiki Fujikane <[email protected]> --------- Signed-off-by: Yoshiki Fujikane <[email protected]> Signed-off-by: Arya Soni <[email protected]>
What this PR does:
as title
Why we need it:
I want to describe how to run k8s multicluster plugin locally.
Which issue(s) this PR fixes:
Part of #5006
Does this PR introduce a user-facing change?: