Skip to content
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

add killercoda example: install karmada through CLI #2

Merged
merged 1 commit into from
Apr 10, 2023

Conversation

JadeFlute0127
Copy link
Contributor

@JadeFlute0127 JadeFlute0127 commented Mar 30, 2023

Signed-off-by: [JadeFlute0127] [1635468471@qq.com]

What this PR does / why we need it:
add killercoda example: install karmada through CLI

Which issue(s) this PR fixes:
Part of #1

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

NONE

@karmada-bot
Copy link
Collaborator

Welcome @JadeFlute0127! It looks like this is your first PR to karmada-io/playground 🎉

@karmada-bot karmada-bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Mar 30, 2023
@jwcesign
Copy link
Member

Please replace the word Fix with Part of.

Copy link
Member

@jwcesign jwcesign left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job! Just some nits.
We currently do not require the killercoda-sample directory.

@JadeFlute0127
Copy link
Contributor Author

I think these problems have been solved~

Copy link
Member

@jwcesign jwcesign left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other looks good to me

karmada-CLI-installtion-and-deploy-example/foreground.sh Outdated Show resolved Hide resolved
karmada-CLI-installtion-and-deploy-example/foreground.sh Outdated Show resolved Hide resolved
karmada-CLI-installtion-and-deploy-example/foreground.sh Outdated Show resolved Hide resolved
karmada-CLI-installtion-and-deploy-example/foreground.sh Outdated Show resolved Hide resolved
karmada-CLI-installtion-and-deploy-example/foreground.sh Outdated Show resolved Hide resolved
karmada-CLI-installtion-and-deploy-example/intro.md Outdated Show resolved Hide resolved
karmada-CLI-installtion-and-deploy-example/intro.md Outdated Show resolved Hide resolved
@jwcesign
Copy link
Member

jwcesign commented Apr 3, 2023

Could you please share the scenario link for your experiment on Killercoda?

@JadeFlute0127
Copy link
Contributor Author

Could you please share the scenario link for your experiment on Killercoda?

https://killercoda.com/zhangdiandian/scenario/karmada-CLI-installtion-and-deploy-example

@karmada-bot karmada-bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Apr 3, 2023
@karmada-bot karmada-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Apr 3, 2023
}

function scpOperations() {
scp installKind.sh root@${member_cluster_ip}:~
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how about the name copyConfigFilesToNode?

```
3. join member clusters(kind-member1 and kind-member2)to karmada cluster

```shell
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried the example, but it requires manual input from you.
How about this one, just click,it will exec the command?
https://killercoda.com/kubevela/course/quick-start/kubevela-first-app
image

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The MD file should specify that the subsequent operations can only be performed after the kind cluster is prepared.

KUBECONFIG_PATH=${KUBECONFIG_PATH:-"${HOME}/.kube"}

function installKind() {
cat << EOF > installKind.sh
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please leave a space of four. other places should be the same

@JadeFlute0127
Copy link
Contributor Author

fix done

@@ -0,0 +1,43 @@
### Background:

1. The kubeconfig files for the Karmada cluster, member1 cluster, and member2 cluster are located in the $HOME/.kube directory. These files are named config, config-member1, and config-member2 respectively.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't usually say Karmada cluster, if you mean the cluster where installing Karmada, I'd prefer to use host cluster.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for pointing out

karmada-CLI-installtion-and-deploy-example/intro.md Outdated Show resolved Hide resolved
@@ -0,0 +1,3 @@
#!/bin/bash

kubectl --kubeconfig=$HOME/.kube/config-member1 config get-contexts kind-member1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a line break, same as in other places.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this comment hasn't been addressed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

@jwcesign
Copy link
Member

jwcesign commented Apr 4, 2023

cc @RainbowMango, could you please review this? Overall, it looks good to me.

@RainbowMango
Copy link
Member

Sure.
/assign

Copy link
Member

@RainbowMango RainbowMango left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally looks good to me. Just some small touches.

  1. I see you name the steps by step1/spep2..., can we give them meaningful names?
  2. Do we need verification for each steps?

@@ -0,0 +1,38 @@
{
"title": "install Karmada cluster through CLI",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't usually say Karmada cluster. How about

Suggested change
"title": "install Karmada cluster through CLI",
"title": "install Karmada through CLI",

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the first question,We define the name of each step in the index. json file。like this:
image

@@ -0,0 +1,38 @@
{
"title": "install Karmada cluster through CLI",
"description": "Installing a Karmada cluster with Kubernetes member through the Karmada CLI",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"description": "Installing a Karmada cluster with Kubernetes member through the Karmada CLI",
"description": "Installing Karmada on Kubernetes through the Karmada CLI",

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

@@ -0,0 +1,3 @@
#!/bin/bash

kubectl --kubeconfig=$HOME/.kube/config-member1 config get-contexts kind-member1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this comment hasn't been addressed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need a new line at end of file.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need a new line at end of this file.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a newline at end of this file.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fix done

@@ -0,0 +1 @@
**The end**
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It'd be great to have a summarize of this playground and make rooms for guiding people to other scenarios.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will add the summary section.
Do you mean to switch to other scenes in Karmada? Other scenarios have not yet been implemented, and the website provides links to other scenarios in Killercoda.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For now, since we don't have any other scenarios yet, so a summary would be fine.
But in the future, we might need to add links to other scenarios for people to experience other features.

@JadeFlute0127
Copy link
Contributor Author

it looks fix done

@jwcesign
Copy link
Member

jwcesign commented Apr 4, 2023

2.Do we need verification for each steps?

I think that the following steps require the completion of the previous steps.

@jwcesign
Copy link
Member

jwcesign commented Apr 4, 2023

/lgtm
cc @RainbowMango for final checking

@karmada-bot karmada-bot added the lgtm Indicates that a PR is ready to be merged. label Apr 4, 2023
@@ -0,0 +1,3 @@
**Summary**

In this scenario, we learned how to initialize the Karmada control plane, join a cluster, and deploy workloads across multiple clusters.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and deploy workloads across multiple clusters

Seems we didn't have the step for deploying workloads, right?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We plan to work on it separately, and it should be completed in the next PR.


Karmada aims to provide turnkey automation for multi-cluster application management in multi-cloud and hybrid cloud scenarios, with key features such as centralized multi-cloud management, high availability, failure recovery, and traffic scheduling.

In this scenario, we will learn how to initialize the Karmada control plane, join clusters, and deploy workloads across multiple clusters.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto.
cc @jwcesign

@RainbowMango RainbowMango added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Apr 4, 2023
@karmada-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

Approval requirements bypassed by manually added approval.

This pull-request has been approved by:

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@karmada-bot karmada-bot removed the lgtm Indicates that a PR is ready to be merged. label Apr 4, 2023
@JadeFlute0127
Copy link
Contributor Author

remove about deploy workload info in intro.md and finish.md


Karmada aims to provide turnkey automation for multi-cluster application management in multi-cloud and hybrid cloud scenarios, with key features such as centralized multi-cloud management, high availability, failure recovery, and traffic scheduling.

In this scenario, we will learn how to initialize the Karmada control plane, join clusters.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this scenario, we will learn how to initialize the Karmada control plane and join clusters.

@@ -0,0 +1,3 @@
**Summary**

In this scenario, we learned how to initialize the Karmada control plane, join a cluster.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this scenario, we learned how to initialize the Karmada control plane and join a cluster.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, thank you for your pointing out, that's a typo.
I think fix done.

Signed-off-by: zhangdiandian <1635468471@qq.com>
@jwcesign
Copy link
Member

jwcesign commented Apr 6, 2023

/lgtm

@karmada-bot karmada-bot added the lgtm Indicates that a PR is ready to be merged. label Apr 6, 2023
Copy link
Member

@RainbowMango RainbowMango left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/hold cancel

@karmada-bot karmada-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 10, 2023
@karmada-bot karmada-bot merged commit d0b6e3b into karmada-io:main Apr 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants