Fix the defaulting for the leader election config map (#1182)

* update go-cmpt to work with 1.14.

Mostly to fix https://github.com/google/go-cmp/issues/167, but we also pinned at some
random commit, rather than at a release version.

* add new pkg

* Make sure we use same versions in pkg and serving

* Fix the defaulting for the leader election cm

This was missing, but now we can provide the default values.

* Fix the defaulting for the leader election cm

This was missing, but now we can provide the default values.
This commit is contained in:
Victor Agababov 2020-03-31 07:23:51 -07:00 committed by GitHub
parent e2ee5bed78
commit 59d2b06f3e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 117 additions and 163 deletions

View File

@ -37,32 +37,35 @@ var (
// NewConfigFromMap returns a Config for the given map, or an error.
func NewConfigFromMap(data map[string]string) (*Config, error) {
config := &Config{
EnabledComponents: sets.NewString(),
}
config := defaultConfig()
if resourceLock := data["resourceLock"]; !validResourceLocks.Has(resourceLock) {
return nil, fmt.Errorf(`resourceLock: invalid value %q: valid values are "leases","configmaps","endpoints"`, resourceLock)
} else {
if resourceLock, ok := data["resourceLock"]; ok {
if !validResourceLocks.Has(resourceLock) {
return nil, fmt.Errorf(`resourceLock: invalid value %q: valid values are "leases","configmaps","endpoints"`, resourceLock)
}
config.ResourceLock = resourceLock
}
if leaseDuration, err := time.ParseDuration(data["leaseDuration"]); err != nil {
return nil, fmt.Errorf("leaseDuration: invalid duration: %q", data["leaseDuration"])
} else {
config.LeaseDuration = leaseDuration
}
if renewDeadline, err := time.ParseDuration(data["renewDeadline"]); err != nil {
return nil, fmt.Errorf("renewDeadline: invalid duration: %q", data["renewDeadline"])
} else {
config.RenewDeadline = renewDeadline
}
if retryPeriod, err := time.ParseDuration(data["retryPeriod"]); err != nil {
return nil, fmt.Errorf("retryPeriod: invalid duration: %q", data["retryPeriod"])
} else {
config.RetryPeriod = retryPeriod
for _, d := range []struct {
key string
val *time.Duration
}{{
"leaseDuration",
&config.LeaseDuration,
}, {
"renewDeadline",
&config.RenewDeadline,
}, {
"retryPeriod",
&config.RetryPeriod,
}} {
if v, ok := data[d.key]; ok {
dur, err := time.ParseDuration(v)
if err != nil {
return nil, fmt.Errorf("%s: invalid duration: %q", d.key, v)
}
*d.val = dur
}
}
// enabledComponents are not validated here, because they are dependent on
@ -82,7 +85,6 @@ func NewConfigFromConfigMap(configMap *corev1.ConfigMap) (*Config, error) {
config := defaultConfig()
return config, nil
}
return NewConfigFromMap(configMap.Data)
}
@ -154,5 +156,5 @@ func UniqueID() (string, error) {
return "", err
}
return (id + "_" + string(uuid.NewUUID())), nil
return id + "_" + string(uuid.NewUUID()), nil
}

View File

@ -22,7 +22,10 @@ import (
"testing"
"time"
"github.com/google/go-cmp/cmp"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/util/sets"
"knative.dev/pkg/kmeta"
)
func okConfig() *Config {
@ -54,111 +57,62 @@ func TestNewConfigMapFromData(t *testing.T) {
data map[string]string
expected *Config
err error
}{
{
name: "disabled but OK config",
data: func() map[string]string {
data := okData()
delete(data, "enabledComponents")
return data
}(),
expected: okConfig(),
},
{
name: "OK config - controller enabled",
data: okData(),
expected: func() *Config {
config := okConfig()
config.EnabledComponents.Insert("controller")
return config
}(),
},
{
name: "missing resourceLock",
data: func() map[string]string {
data := okData()
delete(data, "resourceLock")
return data
}(),
err: errors.New(`resourceLock: invalid value "": valid values are "leases","configmaps","endpoints"`),
},
{
name: "invalid resourceLock",
data: func() map[string]string {
data := okData()
data["resourceLock"] = "flarps"
return data
}(),
err: errors.New(`resourceLock: invalid value "flarps": valid values are "leases","configmaps","endpoints"`),
},
{
name: "missing leaseDuration",
data: func() map[string]string {
data := okData()
delete(data, "leaseDuration")
return data
}(),
err: errors.New(`leaseDuration: invalid duration: ""`),
},
{
name: "invalid leaseDuration",
data: func() map[string]string {
data := okData()
data["leaseDuration"] = "flops"
return data
}(),
err: errors.New(`leaseDuration: invalid duration: "flops"`),
},
{
name: "missing renewDeadline",
data: func() map[string]string {
data := okData()
delete(data, "renewDeadline")
return data
}(),
err: errors.New(`renewDeadline: invalid duration: ""`),
},
{
name: "invalid renewDeadline",
data: func() map[string]string {
data := okData()
data["renewDeadline"] = "flops"
return data
}(),
err: errors.New(`renewDeadline: invalid duration: "flops"`),
},
{
name: "missing retryPeriod",
data: func() map[string]string {
data := okData()
delete(data, "retryPeriod")
return data
}(),
err: errors.New(`retryPeriod: invalid duration: ""`),
},
{
name: "invalid retryPeriod",
data: func() map[string]string {
data := okData()
data["retryPeriod"] = "flops"
return data
}(),
err: errors.New(`retryPeriod: invalid duration: "flops"`),
},
}
}{{
name: "disabled but OK config",
data: func() map[string]string {
data := okData()
delete(data, "enabledComponents")
return data
}(),
expected: okConfig(),
}, {
name: "OK config - controller enabled",
data: okData(),
expected: func() *Config {
config := okConfig()
config.EnabledComponents.Insert("controller")
return config
}(),
}, {
name: "invalid resourceLock",
data: kmeta.UnionMaps(okData(), map[string]string{
"resourceLock": "flarps",
}),
err: errors.New(`resourceLock: invalid value "flarps": valid values are "leases","configmaps","endpoints"`),
}, {
name: "invalid leaseDuration",
data: kmeta.UnionMaps(okData(), map[string]string{
"leaseDuration": "flops",
}),
err: errors.New(`leaseDuration: invalid duration: "flops"`),
}, {
name: "invalid renewDeadline",
data: kmeta.UnionMaps(okData(), map[string]string{
"renewDeadline": "flops",
}),
err: errors.New(`renewDeadline: invalid duration: "flops"`),
}, {
name: "invalid retryPeriod",
data: kmeta.UnionMaps(okData(), map[string]string{
"retryPeriod": "flops",
}),
err: errors.New(`retryPeriod: invalid duration: "flops"`),
}}
for i := range cases {
tc := cases[i]
actualConfig, actualErr := NewConfigFromMap(tc.data)
if !reflect.DeepEqual(tc.err, actualErr) {
t.Errorf("%v: expected error %v, got %v", tc.name, tc.err, actualErr)
continue
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
actualConfig, actualErr := NewConfigFromConfigMap(
&corev1.ConfigMap{
Data: tc.data,
})
if !reflect.DeepEqual(tc.err, actualErr) {
t.Fatalf("Error = %v, want: %v", actualErr, tc.err)
}
if !reflect.DeepEqual(tc.expected, actualConfig) {
t.Errorf("%v: expected config:\n%+v\ngot:\n%+v", tc.name, tc.expected, actualConfig)
continue
}
if got, want := actualConfig, tc.expected; !cmp.Equal(got, want) {
t.Errorf("Config = %#v, want: %#v, diff(-want,+got):\n%s", got, want, cmp.Diff(want, got))
}
})
}
}
@ -167,44 +121,42 @@ func TestGetComponentConfig(t *testing.T) {
name string
config Config
expected ComponentConfig
}{
{
name: "component enabled",
config: Config{
ResourceLock: "leases",
LeaseDuration: 15 * time.Second,
RenewDeadline: 10 * time.Second,
RetryPeriod: 2 * time.Second,
EnabledComponents: sets.NewString("component"),
},
expected: ComponentConfig{
LeaderElect: true,
ResourceLock: "leases",
LeaseDuration: 15 * time.Second,
RenewDeadline: 10 * time.Second,
RetryPeriod: 2 * time.Second,
},
}{{
name: "component enabled",
config: Config{
ResourceLock: "leases",
LeaseDuration: 15 * time.Second,
RenewDeadline: 10 * time.Second,
RetryPeriod: 2 * time.Second,
EnabledComponents: sets.NewString("component"),
},
{
name: "component disabled",
config: Config{
ResourceLock: "leases",
LeaseDuration: 15 * time.Second,
RenewDeadline: 10 * time.Second,
RetryPeriod: 2 * time.Second,
EnabledComponents: sets.NewString("not-the-component"),
},
expected: ComponentConfig{
LeaderElect: false,
},
expected: ComponentConfig{
LeaderElect: true,
ResourceLock: "leases",
LeaseDuration: 15 * time.Second,
RenewDeadline: 10 * time.Second,
RetryPeriod: 2 * time.Second,
},
}
}, {
name: "component disabled",
config: Config{
ResourceLock: "leases",
LeaseDuration: 15 * time.Second,
RenewDeadline: 10 * time.Second,
RetryPeriod: 2 * time.Second,
EnabledComponents: sets.NewString("not-the-component"),
},
expected: ComponentConfig{
LeaderElect: false,
},
}}
for i := range cases {
tc := cases[i]
actual := tc.config.GetComponentConfig("component")
if !reflect.DeepEqual(tc.expected, actual) {
t.Errorf("%v: expected:\n%+v\ngot:\n%+v", tc.name, tc.expected, actual)
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
actual := tc.config.GetComponentConfig("component")
if got, want := actual, tc.expected; !cmp.Equal(got, want) {
t.Errorf("Incorrect config: diff(-want,+got):\n%s", cmp.Diff(want, got))
}
})
}
}