Skip to content

Use configmap parser library from pkg. #7935

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

Merged
knative-prow-robot merged 1 commit into from
May 16, 2020
Merged

Use configmap parser library from pkg. #7935

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
115 changes: 37 additions & 78 deletions pkg/apis/config/defaults.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,14 @@ import (
"fmt"
"io/ioutil"
"math"
"strconv"
"strings"
"text/template"

corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

"knative.dev/pkg/apis"
cm "knative.dev/pkg/configmap"
Copy link
Contributor

@julz julz May 15, 2020

Choose a reason for hiding this comment

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

not wild about the import rename here, but I see why we're doing it so 🤷.

)

const (
Expand Down Expand Up @@ -74,46 +73,23 @@ func defaultConfig() *Defaults {
func NewDefaultsConfigFromMap(data map[string]string) (*Defaults, error) {
nc := defaultConfig()

// Process bool fields.
for _, b := range []struct {
key string
field *bool
}{{
key: "enable-multi-container",
field: &nc.EnableMultiContainer,
}, {
key: "allow-container-concurrency-zero",
field: &nc.AllowContainerConcurrencyZero,
}} {
if raw, ok := data[b.key]; ok {
*b.field = strings.EqualFold(raw, "true")
}
}
if err := cm.Parse(data,
cm.AsString("container-name-template", &nc.UserContainerNameTemplate),

// Process int64 fields.
for _, i64 := range []struct {
key string
field *int64
}{{
key: "revision-timeout-seconds",
field: &nc.RevisionTimeoutSeconds,
}, {
key: "max-revision-timeout-seconds",
field: &nc.MaxRevisionTimeoutSeconds,
}, {
key: "container-concurrency",
field: &nc.ContainerConcurrency,
}, {
key: "container-concurrency-max-limit",
field: &nc.ContainerConcurrencyMaxLimit,
}} {
if raw, ok := data[i64.key]; ok {
val, err := strconv.ParseInt(raw, 10, 64)
if err != nil {
return nil, err
}
*i64.field = val
}
cm.AsBool("enable-multi-container", &nc.EnableMultiContainer),
cm.AsBool("allow-container-concurrency-zero", &nc.AllowContainerConcurrencyZero),

cm.AsInt64("revision-timeout-seconds", &nc.RevisionTimeoutSeconds),
cm.AsInt64("max-revision-timeout-seconds", &nc.MaxRevisionTimeoutSeconds),
cm.AsInt64("container-concurrency", &nc.ContainerConcurrency),
cm.AsInt64("container-concurrency-max-limit", &nc.ContainerConcurrencyMaxLimit),

asQuantity("revision-cpu-request", &nc.RevisionCPURequest),
asQuantity("revision-memory-request", &nc.RevisionMemoryRequest),
asQuantity("revision-cpu-limit", &nc.RevisionCPULimit),
asQuantity("revision-memory-limit", &nc.RevisionMemoryLimit),
); err != nil {
return nil, err
}

if nc.RevisionTimeoutSeconds > nc.MaxRevisionTimeoutSeconds {
Expand All @@ -128,44 +104,13 @@ func NewDefaultsConfigFromMap(data map[string]string) (*Defaults, error) {
nc.ContainerConcurrency, 0, nc.ContainerConcurrencyMaxLimit, "container-concurrency")
}

// Process resource quantity fields
for _, rsrc := range []struct {
key string
field **resource.Quantity
}{{
key: "revision-cpu-request",
field: &nc.RevisionCPURequest,
}, {
key: "revision-memory-request",
field: &nc.RevisionMemoryRequest,
}, {
key: "revision-cpu-limit",
field: &nc.RevisionCPULimit,
}, {
key: "revision-memory-limit",
field: &nc.RevisionMemoryLimit,
}} {
if raw, ok := data[rsrc.key]; !ok {
*rsrc.field = nil
} else if val, err := resource.ParseQuantity(raw); err != nil {
return nil, err
} else {
*rsrc.field = &val
}
tmpl, err := template.New("user-container").Parse(nc.UserContainerNameTemplate)
if err != nil {
return nil, err
}

if raw, ok := data["container-name-template"]; ok {
tmpl, err := template.New("user-container").Parse(raw)
if err != nil {
return nil, err
}
// Check that the template properly applies to ObjectMeta.
if err := tmpl.Execute(ioutil.Discard, metav1.ObjectMeta{}); err != nil {
return nil, fmt.Errorf("error executing template: %w", err)
}
// We store the raw template because we run deepcopy-gen on the
// config and that doesn't copy nicely.
nc.UserContainerNameTemplate = raw
// Check that the template properly applies to ObjectMeta.
if err := tmpl.Execute(ioutil.Discard, metav1.ObjectMeta{}); err != nil {
return nil, fmt.Errorf("error executing template: %w", err)
}

return nc, nil
Expand Down Expand Up @@ -214,3 +159,17 @@ func (d *Defaults) UserContainerName(ctx context.Context) string {
}
return buf.String()
}

// asQuantity parses the value at key as a *resource.Quantity into the target, if it exists.
func asQuantity(key string, target **resource.Quantity) cm.ParseFunc {
return func(data map[string]string) error {
if raw, ok := data[key]; !ok {
*target = nil
} else if val, err := resource.ParseQuantity(raw); err != nil {
return err
} else {
*target = &val
}
return nil
}
}
131 changes: 24 additions & 107 deletions pkg/autoscaler/config/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,9 @@ package config

import (
"fmt"
"strconv"
"strings"
"time"

cm "knative.dev/pkg/configmap"
"knative.dev/serving/pkg/apis/autoscaling"

corev1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -118,83 +117,31 @@ func defaultConfig() *Config {
func NewConfigFromMap(data map[string]string) (*Config, error) {
lc := defaultConfig()

// Process bool fields.
for _, b := range []struct {
key string
field *bool
}{
{
key: "enable-scale-to-zero",
field: &lc.EnableScaleToZero,
},
{
key: "enable-graceful-scaledown",
field: &lc.EnableGracefulScaledown,
}, {
key: "allow-zero-initial-scale",
field: &lc.AllowZeroInitialScale,
}} {
if raw, ok := data[b.key]; ok {
*b.field = strings.EqualFold(raw, "true")
}
}
if err := cm.Parse(data,
cm.AsString("pod-autoscaler-class", &lc.PodAutoscalerClass),

// Process Float64 fields
for _, f64 := range []struct {
key string
field *float64
}{{
key: "max-scale-up-rate",
field: &lc.MaxScaleUpRate,
}, {
key: "max-scale-down-rate",
field: &lc.MaxScaleDownRate,
}, {
key: "container-concurrency-target-percentage",
field: &lc.ContainerConcurrencyTargetFraction,
}, {
key: "container-concurrency-target-default",
field: &lc.ContainerConcurrencyTargetDefault,
}, {
key: "requests-per-second-target-default",
field: &lc.RPSTargetDefault,
}, {
key: "target-burst-capacity",
field: &lc.TargetBurstCapacity,
}, {
key: "panic-window-percentage",
field: &lc.PanicWindowPercentage,
}, {
key: "activator-capacity",
field: &lc.ActivatorCapacity,
}, {
key: "panic-threshold-percentage",
field: &lc.PanicThresholdPercentage,
}} {
if raw, ok := data[f64.key]; ok {
val, err := strconv.ParseFloat(raw, 64)
if err != nil {
return nil, err
}
*f64.field = val
}
}
cm.AsBool("enable-scale-to-zero", &lc.EnableScaleToZero),
cm.AsBool("enable-graceful-scaledown", &lc.EnableGracefulScaledown),
cm.AsBool("allow-zero-initial-scale", &lc.AllowZeroInitialScale),

cm.AsFloat64("max-scale-up-rate", &lc.MaxScaleUpRate),
cm.AsFloat64("max-scale-down-rate", &lc.MaxScaleDownRate),
cm.AsFloat64("container-concurrency-target-percentage", &lc.ContainerConcurrencyTargetFraction),
cm.AsFloat64("container-concurrency-target-default", &lc.ContainerConcurrencyTargetDefault),
cm.AsFloat64("requests-per-second-target-default", &lc.RPSTargetDefault),
cm.AsFloat64("target-burst-capacity", &lc.TargetBurstCapacity),
cm.AsFloat64("panic-window-percentage", &lc.PanicWindowPercentage),
cm.AsFloat64("activator-capacity", &lc.ActivatorCapacity),
cm.AsFloat64("panic-threshold-percentage", &lc.PanicThresholdPercentage),

cm.AsInt32("initial-scale", &lc.InitialScale),

// Process int fields
for _, i := range []struct {
key string
field *int32
}{{
key: "initial-scale",
field: &lc.InitialScale,
}} {
if raw, ok := data[i.key]; ok {
val, err := strconv.ParseInt(raw, 10, 32)
if err != nil {
return nil, err
}
*i.field = int32(val)
}
cm.AsDuration("stable-window", &lc.StableWindow),
cm.AsDuration("scale-to-zero-grace-period", &lc.ScaleToZeroGracePeriod),
cm.AsDuration("scale-to-zero-pod-retention-period", &lc.ScaleToZeroPodRetentionPeriod),
cm.AsDuration("tick-interval", &lc.TickInterval),
); err != nil {
return nil, fmt.Errorf("failed to parse data: %w", err)
}

// Adjust % ⇒ fractions: for legacy reasons we allow values in the
Expand All @@ -205,36 +152,6 @@ func NewConfigFromMap(data map[string]string) (*Config, error) {
lc.ContainerConcurrencyTargetFraction /= 100.0
}

// Process Duration fields
for _, dur := range []struct {
key string
field *time.Duration
}{{
key: "stable-window",
field: &lc.StableWindow,
}, {
key: "scale-to-zero-grace-period",
field: &lc.ScaleToZeroGracePeriod,
}, {
key: "scale-to-zero-pod-retention-period",
field: &lc.ScaleToZeroPodRetentionPeriod,
}, {
key: "tick-interval",
field: &lc.TickInterval,
}} {
if raw, ok := data[dur.key]; ok {
val, err := time.ParseDuration(raw)
if err != nil {
return nil, err
}
*dur.field = val
}
}

if pac, ok := data["pod-autoscaler-class"]; ok {
lc.PodAutoscalerClass = pac
}

return validate(lc)
}

Expand Down
52 changes: 35 additions & 17 deletions pkg/deployment/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,14 @@ limitations under the License.
package deployment

import (
"errors"
"fmt"
"strings"
"time"

corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/util/sets"

cm "knative.dev/pkg/configmap"
)

const (
Expand Down Expand Up @@ -53,25 +54,19 @@ func defaultConfig() *Config {
// NewConfigFromMap creates a DeploymentConfig from the supplied Map
func NewConfigFromMap(configMap map[string]string) (*Config, error) {
nc := defaultConfig()
qsideCarImage, ok := configMap[QueueSidecarImageKey]
if !ok {
return nil, errors.New("queue sidecar image is missing")
}
nc.QueueSidecarImage = qsideCarImage

if pd, ok := configMap[ProgressDeadlineKey]; ok {
v, err := time.ParseDuration(pd)
if err != nil {
return nil, fmt.Errorf("error parsing %s=%s as duration, %w", ProgressDeadlineKey, pd, err)
} else if v <= 0 {
return nil, fmt.Errorf("%s cannot be non-positive duration, was %v", ProgressDeadlineKey, v)
}
nc.ProgressDeadline = v

if err := cm.Parse(configMap,
asRequiredString(QueueSidecarImageKey, &nc.QueueSidecarImage),
cm.AsDuration(ProgressDeadlineKey, &nc.ProgressDeadline),
asStringSet(registriesSkippingTagResolvingKey, &nc.RegistriesSkippingTagResolving),
); err != nil {
return nil, err
}

if registries, ok := configMap[registriesSkippingTagResolvingKey]; ok {
nc.RegistriesSkippingTagResolving = sets.NewString(strings.Split(registries, ",")...)
if nc.ProgressDeadline <= 0 {
return nil, fmt.Errorf("ProgressDeadline cannot be a non-positive duration, was %v", nc.ProgressDeadline)
}

return nc, nil
}

Expand All @@ -93,3 +88,26 @@ type Config struct {
// be ready before considering it failed.
ProgressDeadline time.Duration
}

// asRequiredString is the same as cm.AsString but errors if the key is not present.
func asRequiredString(key string, target *string) cm.ParseFunc {
return func(data map[string]string) error {
if raw, ok := data[key]; ok {
*target = raw
} else {
return fmt.Errorf("%q is missing", key)
}
return nil
}
}

// asStringSet parses the value at key as a sets.String (splitting at ',') into the
// target, if it exists.
func asStringSet(key string, target *sets.String) cm.ParseFunc {
return func(data map[string]string) error {
if raw, ok := data[key]; ok {
*target = sets.NewString(strings.Split(raw, ",")...)
}
return nil
}
}
Loading