-
Notifications
You must be signed in to change notification settings - Fork 408
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
fix and enhancement during my first try, hope it helps #306
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -104,7 +104,9 @@ spec: | |
serviceAccountName: __project_prefix__-controller-manager | ||
hostNetwork: true | ||
tolerations: | ||
- operator: "Exists" | ||
- key: "node-role.kubernetes.io/master" | ||
effect: "" | ||
operator: "Exists" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please comment the reason of adding There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same as previous one |
||
affinity: | ||
nodeAffinity: | ||
# we prefer allocating ecm on cloud node | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -280,6 +280,11 @@ func (co *ConvertOptions) RunConvert() (err error) { | |
edgeNodeNames = append(edgeNodeNames, node.GetName()) | ||
} | ||
|
||
if len(co.CloudNodes) < 1 { | ||
klog.Errorf("At least one cloud node should be provided!") | ||
return | ||
} | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe check There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, you are right |
||
// 3. deploy yurt controller manager | ||
// create a service account for yurt-controller-manager | ||
err = kubeutil.CreateServiceAccountFromYaml(co.clientSet, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -144,7 +144,9 @@ spec: | |
serviceAccountName: yurt-controller-manager | ||
hostNetwork: true | ||
tolerations: | ||
- operator: "Exists" | ||
- key: "node-role.kubernetes.io/master" | ||
effect: "" | ||
operator: "Exists" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please comment the reason of adding key: "node-role.kubernetes.io/master"? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same as previous one |
||
affinity: | ||
nodeAffinity: | ||
# we prefer allocating ecm on cloud node | ||
|
@@ -159,6 +161,7 @@ spec: | |
containers: | ||
- name: yurt-controller-manager | ||
image: {{.image}} | ||
imagePullPolicy: IfNotPresent | ||
command: | ||
- yurt-controller-manager | ||
` | ||
|
@@ -184,7 +187,7 @@ spec: | |
containers: | ||
- name: yurtctl-servant | ||
image: {{.yurtctl_servant_image}} | ||
imagePullPolicy: Always | ||
imagePullPolicy: IfNotPresent | ||
command: | ||
- /bin/sh | ||
- -c | ||
|
@@ -229,7 +232,7 @@ spec: | |
containers: | ||
- name: yurtctl-servant | ||
image: {{.yurtctl_servant_image}} | ||
imagePullPolicy: Always | ||
imagePullPolicy: IfNotPresent | ||
command: | ||
- /bin/sh | ||
- -c | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,62 @@ | ||
#!/usr/bin/env bash | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you can exec There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK, I will remove it from this PR, it is kind of an enhancement, especially for local development, you can build and deliver, then load and deploy with one command |
||
# author: joe.zheng | ||
# version: 21.5.17 | ||
|
||
set -e | ||
|
||
repo="${REL_REPO:-openyurt}" | ||
arch="${REL_ARCH:-amd64}" | ||
tag="${REL_TAG:-latest}" | ||
out="_output/local/bin/linux" | ||
saved="$repo-release" | ||
loads="$saved/load" | ||
images=" | ||
yurthub | ||
yurt-controller-manager | ||
yurtctl-servant | ||
yurt-tunnel-server | ||
yurt-tunnel-agent | ||
" | ||
|
||
echo "build images" | ||
make release ARCH=$arch REPO=$repo GIT_VERSION=$tag | ||
|
||
echo "clear $saved" | ||
rm -rf $saved | ||
mkdir -p $saved | ||
|
||
echo "copy yurtctl" | ||
cp $out/$arch/yurtctl $saved | ||
|
||
echo "save images" | ||
for i in $images; do | ||
old="$repo/$i:$tag-$arch" | ||
new="$repo/$i:$tag" | ||
echo "tag $old to $new" | ||
docker tag $old $new | ||
echo "saving $new" | ||
docker save $new | gzip > $saved/$i.tar.gz | ||
done | ||
|
||
echo "create $loads" | ||
cat <<'EOF' > $loads | ||
#!/usr/bin/env bash | ||
# author: joe.zheng | ||
# version: 21.5.17 | ||
|
||
set -e | ||
|
||
cd $(dirname $0) | ||
|
||
images="$(ls *.tar.gz)" | ||
echo "load images:" | ||
for i in $images; do | ||
echo "loading $i" | ||
docker load -i $i | ||
done | ||
echo "done" | ||
EOF | ||
|
||
chmod a+x $loads | ||
|
||
echo "done" |
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 original
operator: "Exists"
can makeyurt-controller-manager
tolerates all of taints. would you comment the reason of addingkey: "node-role.kubernetes.io/master"
?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.
without explicit blank assignment to
effect
, it can't schedule to master node as I verified, and maybe the original intention is to tolerate any taints, in that case we can assign key as "", but in my opinion strict and explicit way is betterThere 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.
In my cluster, use
operator:"Exists"
yurtctl-controller-manager
could schedule to master node.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.
here is my previous logs, after apply the change, issue fixed
but with the latest code, there is no such issue
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 that
operator: "Exists"
toleration maybe added recently, so the above error is fixed. andoperator: "Exists"
can tolerate all of taints, so it's more suitable for yurt-controller-manager.