Skip to content

Conversation

@xagent003
Copy link
Contributor

@xagent003 xagent003 commented Nov 14, 2022

Just mvoed some functions around, changed ordering. Moved the flat nodelet config struct and generation to a new file called config.go

@xagent003 xagent003 force-pushed the private/arjun/corereorg1 branch from d935416 to 0147ca8 Compare November 14, 2022 23:15
@xagent003 xagent003 changed the title Minor code reorg Minor nodeletctl code reorg Nov 14, 2022
@xagent003 xagent003 force-pushed the private/arjun/corereorg1 branch from 0147ca8 to 02fc4ff Compare November 14, 2022 23:16
ServicesCidr string
}

func setNodeletClusterCfg(cfg *BootstrapConfig, nodelet *NodeletConfig) {

Choose a reason for hiding this comment

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

Why not just return the config object instead of pointer? It makes the function much more readable from the definition.

if _, err := os.Stat(nodeStateDir); os.IsNotExist(err) {
zap.S().Infof("Creating node state dir: %s\n", nodeStateDir)
if err := os.MkdirAll(nodeStateDir, 0777); err != nil {
return "", fmt.Errorf("Failed to create node state dir for host %s: %s", host.HostId, err)

Choose a reason for hiding this comment

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

Error strings should not start with upper case letters. General go convention.

func GenNodeletConfigLocal(host *NodeletConfig, templateName string) (string, error) {
nodeStateDir := filepath.Join(ClusterStateDir, host.ClusterId, host.HostId)
if _, err := os.Stat(nodeStateDir); os.IsNotExist(err) {
zap.S().Infof("Creating node state dir: %s\n", nodeStateDir)

Choose a reason for hiding this comment

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

\n should not be needed.

@mithilarun mithilarun removed their request for review October 5, 2023 14:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants