Feature/support ipv6 dual #1537

VinceCui · 2022-06-22T08:33:49Z

Describe what this PR does / why we need it

支持ipv6双栈模式、ipv6only模式的部署，以扩展Sealer应对的场景

Does this pull request fix one issue?

Describe how you did it

Describe how to verify it

Special notes for reviews

2. Support choose log color mode. 3. Support silence usage, because usage info is noisy in some cases; 4. Silence error, because sealer has already print error itself. 5. Use logrus to print the final error. Signed-off-by: huaiyou <[email protected]>

Fix register plugin bug; Support ipv6 dual stack; TODO: seautil route need to support dual stack.

Fix nil pointer bug.

Signed-off-by: huaiyou <[email protected]>

# Conflicts: # pkg/env/env.go

# Conflicts: # vendor/github.com/sealyun/lvscare/care/care.go # vendor/modules.txt

…6-dual # Conflicts: # apply/apply.go # apply/run.go # apply/scale.go # cmd/sealer/cmd/upgrade.go

Skip add existing node.

…6-dual # Conflicts: # cmd/sealer/cmd/root.go # vendor/github.com/sealyun/lvscare/care/care.go # vendor/modules.txt

codecov-commenter · 2022-06-22T08:58:27Z

Codecov Report

Merging #1537 (e551763) into main (5c4d885) will decrease coverage by 0.06%.
The diff coverage is 5.04%.

@@            Coverage Diff             @@
##             main    #1537      +/-   ##
==========================================
- Coverage   10.86%   10.80%   -0.07%     
==========================================
  Files          88       88              
  Lines        8015     8060      +45     
==========================================
  Hits          871      871              
- Misses       7037     7082      +45     
  Partials      107      107

Impacted Files	Coverage Δ
apply/apply.go	`0.00% <0.00%> (ø)`
apply/run.go	`0.00% <0.00%> (ø)`
apply/scale.go	`6.87% <0.00%> (-0.88%)`	⬇️
apply/validate.go	`86.36% <ø> (-1.14%)`	⬇️
pkg/ipvs/ipvs.go	`0.00% <0.00%> (ø)`
pkg/plugin/etcd_backup_plugin.go	`4.54% <0.00%> (ø)`
pkg/plugin/hostname_plugin.go	`18.75% <0.00%> (ø)`
pkg/plugin/plugins.go	`0.00% <0.00%> (ø)`
pkg/plugin/shell_plugin.go	`23.07% <0.00%> (ø)`
pkg/runtime/init.go	`0.00% <0.00%> (ø)`
... and 21 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5c4d885...e551763. Read the comment docs.

VinceCui · 2022-06-22T09:12:41Z

The DCO check fails, because some commit is not signed, but when i rebase, many conflict occurs..

So, please ignore this check and continue REVIEW.

allencloud · 2022-06-22T09:29:27Z

Since I merged #1500, conflict happens. Please rebase. Thanks. @VinceCui

apply/apply.go

allencloud · 2022-06-22T09:37:46Z

apply/apply.go

+		}
+	}
+
+	if hasBoth := hasIPV4 && hasIPV6; hasBoth {


if hasIPV4 && hasIPV6 {
blabla
}

pkg/env/env.go

…6-dual # Conflicts: # apply/apply.go # apply/run.go # apply/scale.go # utils/net/iputils_test.go

allencloud · 2022-06-23T05:06:21Z

apply/apply.go

 		ImageManager:        imgSvc,
 		ClusterImageMounter: mounter,
 		ImageStore:          is,
 	}, nil
 }
+
+func checkAllHostsSameFamily(nodeList []string) error {


This is the validate function, you could add them into validate.go . Actually It seems to have nothing to do with the main apply procedure. @VinceCui

allencloud · 2022-06-23T05:21:11Z

pkg/cert/kube_certs.go

-	if err != nil {
-		return nil, err
+
+	for _, cidr := range strings.Split(SvcCIDR, ",") {


Same issue, this should be validated at the very beginning part, or validation layer. It is not suggested that we validate this in the underlying implementation layer. Should use Defensive Programming.

allencloud · 2022-06-23T05:21:40Z

@VinceCui

I have several suggestions here after review:

put all validation all of codes for IPv4 and IPv6 into a validation layer before the main processor, such as before NewDefaultApplier. We could unified the validation of input, no matter the command ARG input or Clustefile content input. When we could achieve main procedure more focused and decoupled inner architecture.

VinceCui · 2022-06-23T10:29:31Z

@allencloud
I agree, but it is a big engineering

ignore Port-10250 in kubeadm

…ow a warning. Fix bug, a node slice should be nil if arg --masters or --workers is empty.

Signed-off-by: huaiyou <[email protected]>

allencloud · 2022-06-29T00:23:06Z

apply/applydriver/local.go

@@ -49,6 +49,7 @@ type Applier struct {
 	Client              *k8s.Client
 	ImageStore          store.ImageStore
 	CurrentClusterInfo  *version.Info
+	Action              string


I am afraid that I do not agree that passing a cmd layer type into an underlying procedure, here is the Applier struct. It makes me feel a bottom struct relies on the upper struct.

Ignore DirAvailable--etc-kubernetes-manifests when join; Explicitly specify the list of nodes to be deleted and to be added; Signed-off-by: huaiyou <[email protected]>

bxy4543 · 2022-07-06T06:39:04Z

pkg/runtime/masters.go

 	InitMaser115Upper           = `kubeadm init --config=%s/etc/kubeadm.yml --upload-certs`
 	JoinMaster115Upper          = "kubeadm join --config=%s/etc/kubeadm.yml"
 	JoinNode115Upper            = "kubeadm join --config=%s/etc/kubeadm.yml"
 	RemoveKubeConfig            = "rm -rf /usr/bin/kube* && rm -rf ~/.kube/"
-	RemoteCleanMasterOrNode     = `if which kubeadm;then kubeadm reset -f %s;fi && \
+	RemoteCleanMasterOrNode     = `systemctl restart docker kubelet;if which kubeadm;then kubeadm reset -f %s;fi && \


Why does the delete operation have to restart the service instead of stopping the service?

bxy4543 · 2022-07-06T06:47:12Z

utils/ssh/ssh.go

-				if err := mergo.Merge(&host.SSH, &cluster.Spec.SSH); err != nil {
-					return nil, err
+				if host.SSH.Pk != "" || host.SSH.Passwd != "" {
+					return NewSSHClient(&host.SSH, false), nil


false --> isStdout

bxy4543 · 2022-07-06T06:47:20Z

utils/ssh/ssh.go

-					return nil, err
-				}
-				return NewSSHClient(&host.SSH, true), nil
+				return NewSSHClient(&cluster.Spec.SSH, false), nil


false --> isStdout

bxy4543 · 2022-07-06T06:47:28Z

utils/ssh/ssh.go

 			}
 		}
 	}
+	if cluster.Spec.SSH.Pk != "" || cluster.Spec.SSH.Passwd != "" {
+		return NewSSHClient(&cluster.Spec.SSH, false), nil


false --> isStdout

bxy4543 · 2022-07-06T06:54:40Z

utils/strings/strings.go

+	if len(newSlice) == 0 {
+		// Sanitize for unit tests so we don't need to distinguish empty array
+		// and nil.
+		newSlice = nil


return nil is better?

bxy4543 · 2022-07-06T06:59:13Z

pkg/runtime/masters.go

@@ -306,10 +307,10 @@ func (k *KubeadmRuntime) Command(version string, name CommandType) (cmd string)
 		return fmt.Sprintf("%s%s%s", v, vlogToStr(k.Vlog), " --ignore-preflight-errors=all")
 	}
 	if name == InitMaster || name == JoinMaster {
-		return fmt.Sprintf("%s%s%s", v, vlogToStr(k.Vlog), " --ignore-preflight-errors=SystemVerification")
+		return fmt.Sprintf("%s%s%s", v, vlogToStr(k.Vlog), " --ignore-preflight-errors=SystemVerification,Port-10250,DirAvailable--etc-kubernetes-manifests")


If kubelet is not started, port 10250 does not need to be ignored

VinceCui and others added 13 commits June 10, 2022 15:05

Support ipv6 dual stack.

7f1df5d

1. Add remote log hook.

e922365

2. Support choose log color mode. 3. Support silence usage, because usage info is noisy in some cases; 4. Silence error, because sealer has already print error itself. 5. Use logrus to print the final error. Signed-off-by: huaiyou <[email protected]>

Support skip mount;

489935e

Fix register plugin bug; Support ipv6 dual stack; TODO: seautil route need to support dual stack.

Fix lvscare support ipv6

8dd413d

Fix nil pointer bug.

Fix sealer run.

2309e05

Use template from go standard library rather than html's

151acf4

Signed-off-by: huaiyou <[email protected]>

Add b64enc and b64dec func

43fbbfd

Signed-off-by: huaiyou <[email protected]>

Merge branch 'fix/go-template' into feature/support-ipv6-dual

0bd5199

# Conflicts: # pkg/env/env.go

Merge branch 'feature/add-remote-logger' into feature/support-ipv6-dual

c5071e6

# Conflicts: # vendor/github.com/sealyun/lvscare/care/care.go # vendor/modules.txt

Merge remote-tracking branch 'upstream/main' into feature/support-ipv…

3ad77d6

…6-dual # Conflicts: # apply/apply.go # apply/run.go # apply/scale.go # cmd/sealer/cmd/upgrade.go

Fix merge ssh config.

d772ed9

Check if node list in clusterfile is same as k8s's;

781a0b9

Skip add existing node.

Merge remote-tracking branch 'upstream/main' into feature/support-ipv…

bc8ae79

…6-dual # Conflicts: # cmd/sealer/cmd/root.go # vendor/github.com/sealyun/lvscare/care/care.go # vendor/modules.txt

github-actions bot added Clusterfile plugin test labels Jun 22, 2022

Agree lint.

0b7a3ab

VinceCui requested review from bxy4543 and allencloud June 22, 2022 09:04

VinceCui linked an issue Jun 22, 2022 that may be closed by this pull request

Support ipv4/ipv6 dual stack. #1415

Open

allencloud reviewed Jun 22, 2022

View reviewed changes

apply/apply.go Show resolved Hide resolved

allencloud reviewed Jun 22, 2022

View reviewed changes

apply/apply.go Outdated

}

}

if hasBoth := hasIPV4 && hasIPV6; hasBoth {

Copy link

Member

allencloud Jun 22, 2022

Choose a reason for hiding this comment

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

if hasIPV4 && hasIPV6 {
blabla
}

allencloud reviewed Jun 22, 2022

View reviewed changes

pkg/env/env.go Outdated Show resolved Hide resolved

Merge remote-tracking branch 'upstream/main' into feature/support-ipv…

6556e68

…6-dual # Conflicts: # apply/apply.go # apply/run.go # apply/scale.go # utils/net/iputils_test.go

github-actions bot removed the test label Jun 23, 2022

allencloud reviewed Jun 23, 2022

View reviewed changes

VinceCui added 4 commits June 25, 2022 23:19

Improve code

48b5043

ignore Port-10250 in kubeadm

Should not do clean work when apply, which is too dangerous, just thr…

11de6f6

…ow a warning. Fix bug, a node slice should be nil if arg --masters or --workers is empty.

Fix join bug.

6cfc40e

Signed-off-by: huaiyou <[email protected]>

Avoid ambiguous cluster operation.

b302642

Signed-off-by: huaiyou <[email protected]>

allencloud reviewed Jun 29, 2022

View reviewed changes

Merge func NewStdoutSSHClient and GetHostSSHClient;

e551763

Ignore DirAvailable--etc-kubernetes-manifests when join; Explicitly specify the list of nodes to be deleted and to be added; Signed-off-by: huaiyou <[email protected]>

bxy4543 reviewed Jul 6, 2022

View reviewed changes

VinceCui closed this Jul 6, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/support ipv6 dual #1537

Feature/support ipv6 dual #1537

VinceCui commented Jun 22, 2022 •

edited by allencloud

Loading

codecov-commenter commented Jun 22, 2022 •

edited

Loading

VinceCui commented Jun 22, 2022

allencloud commented Jun 22, 2022

allencloud Jun 22, 2022

allencloud Jun 23, 2022

allencloud Jun 23, 2022

allencloud commented Jun 23, 2022

VinceCui commented Jun 23, 2022

allencloud Jun 29, 2022

bxy4543 Jul 6, 2022

bxy4543 Jul 6, 2022

bxy4543 Jul 6, 2022

bxy4543 Jul 6, 2022

bxy4543 Jul 6, 2022

bxy4543 Jul 6, 2022

Feature/support ipv6 dual #1537

Feature/support ipv6 dual #1537

Conversation

VinceCui commented Jun 22, 2022 • edited by allencloud Loading

Describe what this PR does / why we need it

Does this pull request fix one issue?

Describe how you did it

Describe how to verify it

Special notes for reviews

codecov-commenter commented Jun 22, 2022 • edited Loading

Codecov Report

VinceCui commented Jun 22, 2022

allencloud commented Jun 22, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

allencloud commented Jun 23, 2022

VinceCui commented Jun 23, 2022

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

VinceCui commented Jun 22, 2022 •

edited by allencloud

Loading

codecov-commenter commented Jun 22, 2022 •

edited

Loading