Repository: knative/pkg
Pull request #1362 (2020-05-28 12:33:12)

Add AsQuantity to configmap parser
moving over [same method from serving config/defaults.go](https://github.com/knative/serving/blob/401e03a/pkg/apis/config/defaults.go#L160) so it can be used in deployment configmap also.

/assign @markusthoemmes 

knative-prow-robot: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by: *<a href="https://github.com/knative/pkg/pull/1362#" title="Author self-approved">julz</a>*
To complete the [pull request process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process), please assign **mdemirhan**
You can assign the PR to them by writing `/assign @mdemirhan` in a comment when ready.

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=knative%2Fpkg).

<details open>
Needs approval from an approver in each of these files:

- **[configmap/OWNERS](https://github.com/knative/pkg/blob/master/configmap/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":["mdemirhan"]} -->
knative-metrics-robot: The following is the coverage report on the affected files.
Say `/test pull-knative-pkg-go-coverage` to re-run this coverage report

File | Old Coverage | New Coverage | Delta
---- |:------------:|:------------:|:-----:
[configmap/parse.go](https://storage.cloud.google.com/knative-prow/pr-logs/pull/knative_pkg/1362/pull-knative-pkg-go-coverage/1265984618624978945/artifacts/line-cov.html#file0) | Do not exist | 100.0% | 

knative-prow-robot: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by: *<a href="https://github.com/knative/pkg/pull/1362#" title="Author self-approved">julz</a>*, *<a href="https://github.com/knative/pkg/pull/1362#pullrequestreview-420063808" title="Approved">markusthoemmes</a>*
To complete the [pull request process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process), please assign **mdemirhan**
You can assign the PR to them by writing `/assign @mdemirhan` in a comment when ready.

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=knative%2Fpkg).

<details open>
Needs approval from an approver in each of these files:

- **[configmap/OWNERS](https://github.com/knative/pkg/blob/master/configmap/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":["mdemirhan"]} -->
knative-prow-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/knative/pkg/pull/1362#" title="Author self-approved">julz</a>*, *<a href="https://github.com/knative/pkg/pull/1362#pullrequestreview-420063808" title="Approved">markusthoemmes</a>*, *<a href="https://github.com/knative/pkg/pull/1362#pullrequestreview-420331345" title="LGTM">vagababov</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=knative%2Fpkg).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[configmap/OWNERS](https://github.com/knative/pkg/blob/master/configmap/OWNERS)~~ [vagababov]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->

Patch:
diff --git a/configmap/parse.go b/configmap/parse.go
index 6479598b7..210d2114e 100644
--- a/configmap/parse.go
+++ b/configmap/parse.go
@@ -22,6 +22,7 @@ import (
 	"strings"
 	"time"
 
+	"k8s.io/apimachinery/pkg/api/resource"
 	"k8s.io/apimachinery/pkg/util/sets"
 )
 
@@ -114,6 +115,21 @@ func AsStringSet(key string, target *sets.String) ParseFunc {
 	}
 }
 
+// AsQuantity parses the value at key as a *resource.Quantity into the target, if it exists
+func AsQuantity(key string, target **resource.Quantity) ParseFunc {
+	return func(data map[string]string) error {
+		if raw, ok := data[key]; ok {
+			val, err := resource.ParseQuantity(raw)
+			if err != nil {
+				return fmt.Errorf("failed to parse %q: %w", key, err)
+			}
+
+			*target = &val
+		}
+		return nil
+	}
+}
+
 // Parse parses the given map using the parser functions passed in.
 func Parse(data map[string]string, parsers ...ParseFunc) error {
 	for _, parse := range parsers {


Test patch:
diff --git a/configmap/parse_test.go b/configmap/parse_test.go
index 14528d351..477dd1488 100644
--- a/configmap/parse_test.go
+++ b/configmap/parse_test.go
@@ -21,6 +21,7 @@ import (
 	"time"
 
 	"github.com/google/go-cmp/cmp"
+	"k8s.io/apimachinery/pkg/api/resource"
 	"k8s.io/apimachinery/pkg/util/sets"
 )
 
@@ -32,9 +33,11 @@ type testConfig struct {
 	f64 float64
 	dur time.Duration
 	set sets.String
+	qua *resource.Quantity
 }
 
 func TestParse(t *testing.T) {
+	fiveHundredM := resource.MustParse("500m")
 	tests := []struct {
 		name      string
 		conf      testConfig
@@ -51,6 +54,7 @@ func TestParse(t *testing.T) {
 			"test-float64":  "1.0",
 			"test-duration": "1m",
 			"test-set":      "a,b,c",
+			"test-quantity": "500m",
 		},
 		want: testConfig{
 			str: "foo.bar",
@@ -60,6 +64,7 @@ func TestParse(t *testing.T) {
 			f64: 1.0,
 			dur: time.Minute,
 			set: sets.NewString("a", "b", "c"),
+			qua: &fiveHundredM,
 		},
 	}, {
 		name: "respect defaults",
@@ -70,6 +75,7 @@ func TestParse(t *testing.T) {
 			i64: 2,
 			f64: 1.0,
 			dur: time.Minute,
+			qua: &fiveHundredM,
 		},
 		want: testConfig{
 			str: "foo.bar",
@@ -78,6 +84,7 @@ func TestParse(t *testing.T) {
 			i64: 2,
 			f64: 1.0,
 			dur: time.Minute,
+			qua: &fiveHundredM,
 		},
 	}, {
 		name: "bool defaults to false",
@@ -111,6 +118,12 @@ func TestParse(t *testing.T) {
 			"test-duration": "foo",
 		},
 		expectErr: true,
+	}, {
+		name: "quantity error",
+		data: map[string]string{
+			"test-quantity": "foo",
+		},
+		expectErr: true,
 	}}
 
 	for _, test := range tests {
@@ -123,6 +136,7 @@ func TestParse(t *testing.T) {
 				AsFloat64("test-float64", &test.conf.f64),
 				AsDuration("test-duration", &test.conf.dur),
 				AsStringSet("test-set", &test.conf.set),
+				AsQuantity("test-quantity", &test.conf.qua),
 			); (err == nil) == test.expectErr {
 				t.Fatal("Failed to parse data:", err)
 			}