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

Potential duplicate code in create cluster pipeline #1689

Closed
lwabish opened this issue Jan 10, 2023 · 2 comments
Closed

Potential duplicate code in create cluster pipeline #1689

lwabish opened this issue Jan 10, 2023 · 2 comments
Labels
bug Something isn't working

Comments

@lwabish
Copy link
Contributor

lwabish commented Jan 10, 2023

What is version of KubeKey has the issue?

master branch HEAD commit id: ec903fe

What is your os environment?

doesn't matter

KubeKey config file

No response

A clear and concise description of what happend.

I was reading code of kubekey about kube-vip, found that there are two lines of KubevipModule in cluster creation pipeline.
Is this duplicate code or an intended implementation?

&loadbalancer.KubevipModule{Skip: !runtime.Cluster.ControlPlaneEndpoint.IsInternalLBEnabledVip()},
&kubernetes.InitKubernetesModule{},
&dns.ClusterDNSModule{},
&kubernetes.StatusModule{},
&kubernetes.JoinNodesModule{},
&loadbalancer.KubevipModule{Skip: !runtime.Cluster.ControlPlaneEndpoint.IsInternalLBEnabledVip()},
&loadbalancer.HaproxyModule{Skip: !runtime.Cluster.ControlPlaneEndpoint.IsInternalLBEnabled()},

Relevant log output

No response

Additional information

No response

@lwabish lwabish added the bug Something isn't working label Jan 10, 2023
@24sama
Copy link
Collaborator

24sama commented Jan 11, 2023

In this KubevipModule , there is some code to control his logic in different cases.

if exist, _ := k.BaseModule.PipelineCache.GetMustBool(common.ClusterExist); exist {
k.Tasks = []task.Interface{
checkVIPAddress,
getInterface,
kubevipManifestNotFirstMaster,
}
} else {
k.Tasks = []task.Interface{
checkVIPAddress,
getInterface,
kubevipManifestOnlyFirstMaster,
}
}

@lwabish
Copy link
Contributor Author

lwabish commented Jan 11, 2023

Got it. Maybe I could add some code comments later to help developers understand this better.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants