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.

Sign up for GitHub

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

tests: fix anti-pattern name in NewEtcdProcessCluster #14825

Merged
merged 1 commit into from
Nov 23, 2022
Merged

tests: fix anti-pattern name in NewEtcdProcessCluster #14825

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion tests/e2e/ctl_v3_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ func testCtlWithOffline(t *testing.T, testFunc func(ctlCtx), testOfflineFunc fun
if !ret.quorum {
ret.cfg = *e2e.ConfigStandalone(ret.cfg)
}
ret.cfg.DisableStrictReconfigCheck = ret.disableStrictReconfigCheck
ret.cfg.StrictReconfigCheck = !ret.disableStrictReconfigCheck
if ret.initialCorruptCheck {
ret.cfg.InitialCorruptCheck = ret.initialCorruptCheck
}
Expand Down
36 changes: 19 additions & 17 deletions tests/framework/e2e/cluster.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ func NewConfigClientTLSCertAuthWithNoCN() *EtcdProcessClusterConfig {
WithClusterSize(1),
WithClientConnType(ClientTLS),
WithClientCertAuthority(true),
WithNoCN(true),
WithCN(false),
)
}

Expand Down Expand Up @@ -152,18 +152,18 @@ type EtcdProcessClusterConfig struct {
Client ClientConfig
IsPeerTLS bool
IsPeerAutoTLS bool
NoCN bool
CN bool

CipherSuites []string

ForceNewCluster bool
InitialToken string
QuotaBackendBytes int64
DisableStrictReconfigCheck bool
Copy link
Member

Choose a reason for hiding this comment

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

No sure we should change it.

Both Disable or No seems to enable it by default. If we switch to CN, I think we should change all the places to DefaultXXXConfig, like DefaultConfig. What do you think?

Copy link
Member

Choose a reason for hiding this comment

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

Looks like DefaultConfig was updated. As this is just an internal test framework I don't see any negative impacts of changing the defaults.

Copy link
Member

Choose a reason for hiding this comment

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

My bad. The WithConfigXYZ always call DefaultConfig. For sure, it looks good.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for taking the time to review, I should have explained it earlier😬

EnableV2 bool
InitialCorruptCheck bool
AuthTokenOpts string
V2deprecation string
ForceNewCluster bool
InitialToken string
QuotaBackendBytes int64
StrictReconfigCheck bool
EnableV2 bool
InitialCorruptCheck bool
AuthTokenOpts string
V2deprecation string

RollingStart bool

Expand All @@ -186,8 +186,10 @@ type EtcdProcessClusterConfig struct {

func DefaultConfig() *EtcdProcessClusterConfig {
return &EtcdProcessClusterConfig{
ClusterSize: 3,
InitialToken: "new",
ClusterSize: 3,
InitialToken: "new",
StrictReconfigCheck: true,
CN: true,
}
}

Expand Down Expand Up @@ -257,16 +259,16 @@ func WithClientRevokeCerts(isClientCRL bool) EPClusterOption {
return func(c *EtcdProcessClusterConfig) { c.Client.RevokeCerts = isClientCRL }
}

func WithNoCN(noCN bool) EPClusterOption {
return func(c *EtcdProcessClusterConfig) { c.NoCN = noCN }
func WithCN(cn bool) EPClusterOption {
return func(c *EtcdProcessClusterConfig) { c.CN = cn }
}

func WithQuotaBackendBytes(bytes int64) EPClusterOption {
return func(c *EtcdProcessClusterConfig) { c.QuotaBackendBytes = bytes }
}

func WithDisableStrictReconfigCheck(disable bool) EPClusterOption {
return func(c *EtcdProcessClusterConfig) { c.DisableStrictReconfigCheck = disable }
func WithStrictReconfigCheck(strict bool) EPClusterOption {
return func(c *EtcdProcessClusterConfig) { c.StrictReconfigCheck = strict }
}

func WithEnableV2(enable bool) EPClusterOption {
Expand Down Expand Up @@ -488,7 +490,7 @@ func (cfg *EtcdProcessClusterConfig) EtcdServerProcessConfig(tb testing.TB, i in
"--quota-backend-bytes", fmt.Sprintf("%d", cfg.QuotaBackendBytes),
)
}
if cfg.DisableStrictReconfigCheck {
if !cfg.StrictReconfigCheck {
args = append(args, "--strict-reconfig-check=false")
}
if cfg.EnableV2 {
Expand Down
2 changes: 1 addition & 1 deletion tests/framework/e2e/curl.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ func CURLPrefixArgs(cfg *EtcdProcessClusterConfig, member EtcdProcess, method st
cmdArgs = append(cmdArgs, "--cacert", CaPath, "--cert", CertPath, "--key", PrivateKeyPath)
acurl = ToTLS(member.Config().Acurl)
} else if cfg.Client.ConnectionType == ClientTLS {
if !cfg.NoCN {
if cfg.CN {
cmdArgs = append(cmdArgs, "--cacert", CaPath, "--cert", CertPath, "--key", PrivateKeyPath)
} else {
cmdArgs = append(cmdArgs, "--cacert", CaPath, "--cert", CertPath3, "--key", PrivateKeyPath3)
Expand Down
2 changes: 1 addition & 1 deletion tests/framework/e2e/e2e.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func (e e2eRunner) NewCluster(ctx context.Context, t testing.TB, opts ...config.
e2eConfig := NewConfig(
WithClusterSize(cfg.ClusterSize),
WithQuotaBackendBytes(cfg.QuotaBackendBytes),
WithDisableStrictReconfigCheck(!cfg.StrictReconfigCheck),
WithStrictReconfigCheck(cfg.StrictReconfigCheck),
WithAuthTokenOpts(cfg.AuthToken),
WithSnapshotCount(cfg.SnapshotCount),
)
Expand Down