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

fix: etcd extra args not passed to etcd #248

Conversation

salasberryfin
Copy link
Contributor

What this PR does / why we need it:

Extra arguments passed to etcd are defined in rke2AgentConfig which is embedded in rke2ServerConfig. The issue occurs when we generate control plane configuration and populate these two structures, here:

func GenerateInitControlPlaneConfig(opts ServerConfigOpts) (*rke2ServerConfig, []bootstrapv1.File, error) {
	if opts.Token == "" {
		return nil, nil, fmt.Errorf("token is required")
	}

	rke2ServerConfig, serverFiles, err := newRKE2ServerConfig(opts)
	if err != nil {
		return nil, nil, fmt.Errorf("failed to generate rke2 server config: %w", err)
	}

	rke2AgentConfig, agentFiles, err := newRKE2AgentConfig(AgentConfigOpts{
		AgentConfig: opts.AgentConfig,
		Client:      opts.Client,
		Ctx:         opts.Ctx,
		Token:       opts.Token,
	})
	if err != nil {
		return nil, nil, fmt.Errorf("failed to generate rke2 agent config: %w", err)
	}

	rke2ServerConfig.rke2AgentConfig = *rke2AgentConfig

	return rke2ServerConfig, append(serverFiles, agentFiles...), nil
}

rke2ServerConfig is initialized first and etcd extra values are successfully passed but then rke2AgentConfig is generated and embedded into rke2ServerConfig which empties the original value of EtcdArgs.

By moving these etcd configuration fields from rke2AgentConfig to rke2ServerConfig we can avoid these being overridden without having to apply changes to the existing logic.

Which issue(s) this PR fixes:
Fixes #234

Special notes for your reviewer:

While reproducing the bug, creating a RKE2ControlPlane with extra arguments for etcd resulted in an incorrect configuration, as validated from one of the control plane nodes:

# cat /etc/rancher/rke2/config.yaml                          
cluster-cidr: 10.45.0.0/16                                                                                                                
cni:                                                                                                                                      
- calico                                                                                                                                  
disable-cloud-controller: true                                                                                                            
service-cidr: 10.46.0.0/16                                                                                                                
tls-san:                                                                                                                                  
- 192.168.48.4                                                                                                                            
token: 7f8ddf486d5dff143dbf8964eb4f48ff 

This aligns with the findings in GenerateInitControlPlaneConfig, where the extra arguments are correctly passed to rke2ServerConfig but then deleted after initializing rke2AgentConfig.

After moving the fields to rke2ServerConfig, the final RKE2 configuration matches the expected result:

# cat /etc/rancher/rke2/config.yaml
cluster-cidr: 10.45.0.0/16
cni:
- calico
disable-cloud-controller: true
etcd-arg:
- quota-backend-bytes=5368709120
- auto-compaction-mode=periodic
- auto-compaction-retention=5h
service-cidr: 10.46.0.0/16
tls-san:
- 192.168.48.4
token: 81a3fd0a57013ff15b71a37bd3274cff

Checklist:

  • squashed commits into logical changes
  • includes documentation
  • adds unit tests
  • adds or updates e2e tests

@salasberryfin salasberryfin added the kind/bug Something isn't working label Jan 23, 2024
@salasberryfin salasberryfin requested a review from a team January 23, 2024 12:38
@salasberryfin salasberryfin merged commit c40b93a into rancher:main Jan 23, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RKE2 bootstrap provider does not actually pass etcd extra args to etcd
4 participants