From cc846c9d330a369b496904f5cfed6cbdc13f878d Mon Sep 17 00:00:00 2001 From: sanposhiho <44139130+sanposhiho@users.noreply.github.com> Date: Thu, 2 Sep 2021 01:46:34 +0900 Subject: [PATCH 1/4] Feature: check for unused template parameters --- .../scheduler_perf/scheduler_perf_test.go | 93 ++++++++++++++++--- 1 file changed, 80 insertions(+), 13 deletions(-) diff --git a/test/integration/scheduler_perf/scheduler_perf_test.go b/test/integration/scheduler_perf/scheduler_perf_test.go index 7ae2a60db4a..3bca2bafdb0 100644 --- a/test/integration/scheduler_perf/scheduler_perf_test.go +++ b/test/integration/scheduler_perf/scheduler_perf_test.go @@ -134,7 +134,63 @@ type workload struct { // Name of the workload. Name string // Values of parameters used in the workloadTemplate. - Params map[string]int + Params Params +} + +type Params struct { + params map[string]int + // isUsed field records whether params is used or not. + isUsed map[string]bool +} + +// UnmarshalJSON is a custom unmarshaler for Params. +// +// from(json): +// { +// "initNodes": 500, +// "initPods": 50 +// } +// +// to: +// Params{ +// params: map[string]int{ +// "intNodes": 500, +// "initPods": 50, +// }, +// isUsed: map[string]bool{}, // empty map +// } +// +func (p *Params) UnmarshalJSON(b []byte) error { + aux := map[string]int{} + + if err := json.Unmarshal(b, &aux); err != nil { + return err + } + + p.params = aux + p.isUsed = map[string]bool{} + return nil +} + +// getParam gets param with key. +func (p Params) getParam(key string) (int, error) { + p.isUsed[key] = true + param, ok := p.params[key] + if ok { + return param, nil + } + return 0, fmt.Errorf("parameter %s is undefined", key) +} + +// unusedParams returns the names of unusedParams +func (w workload) unusedParams() []string { + var ret []string + for name := range w.Params.params { + if !w.Params.isUsed[name] { + ret = append(ret, name) + } + } + return ret } // op is a dummy struct which stores the real op in itself. @@ -227,9 +283,10 @@ func (*createNodesOp) collectsMetrics() bool { func (cno createNodesOp) patchParams(w *workload) (realOp, error) { if cno.CountParam != "" { - var ok bool - if cno.Count, ok = w.Params[cno.CountParam[1:]]; !ok { - return nil, fmt.Errorf("parameter %s is undefined", cno.CountParam) + var err error + cno.Count, err = w.Params.getParam(cno.CountParam[1:]) + if err != nil { + return nil, err } } return &cno, (&cno).isValid(false) @@ -268,9 +325,10 @@ func (*createNamespacesOp) collectsMetrics() bool { func (cmo createNamespacesOp) patchParams(w *workload) (realOp, error) { if cmo.CountParam != "" { - var ok bool - if cmo.Count, ok = w.Params[cmo.CountParam[1:]]; !ok { - return nil, fmt.Errorf("parameter %s is undefined", cmo.CountParam) + var err error + cmo.Count, err = w.Params.getParam(cmo.CountParam[1:]) + if err != nil { + return nil, err } } return &cmo, (&cmo).isValid(false) @@ -327,9 +385,10 @@ func (cpo *createPodsOp) collectsMetrics() bool { func (cpo createPodsOp) patchParams(w *workload) (realOp, error) { if cpo.CountParam != "" { - var ok bool - if cpo.Count, ok = w.Params[cpo.CountParam[1:]]; !ok { - return nil, fmt.Errorf("parameter %s is undefined", cpo.CountParam) + var err error + cpo.Count, err = w.Params.getParam(cpo.CountParam[1:]) + if err != nil { + return nil, err } } return &cpo, (&cpo).isValid(false) @@ -368,9 +427,10 @@ func (cpso *createPodSetsOp) collectsMetrics() bool { func (cpso createPodSetsOp) patchParams(w *workload) (realOp, error) { if cpso.CountParam != "" { - var ok bool - if cpso.Count, ok = w.Params[cpso.CountParam[1:]]; !ok { - return nil, fmt.Errorf("parameter %s is undefined", cpso.CountParam) + var err error + cpso.Count, err = w.Params.getParam(cpso.CountParam[1:]) + if err != nil { + return nil, err } } return &cpso, (&cpso).isValid(true) @@ -753,6 +813,13 @@ func runWorkload(b *testing.B, tc *testCase, w *workload) []DataItem { b.Fatalf("op %d: invalid op %v", opIndex, concreteOp) } } + + // check unused params and inform users + unusedParams := w.unusedParams() + for _, p := range unusedParams { + b.Logf("the parameter %s is defined on workload %s, but it is unused. Please make sure there are no typos.", p, w.Name) + } + // Some tests have unschedulable pods. Do not add an implicit barrier at the // end as we do not want to wait for them. return dataItems From 24643c67d586925f5e7a2deea70eb36ab56f94db Mon Sep 17 00:00:00 2001 From: sanposhiho <44139130+sanposhiho@users.noreply.github.com> Date: Thu, 9 Sep 2021 07:10:37 +0900 Subject: [PATCH 2/4] Fix: make struct un-exported --- .../scheduler_perf/scheduler_perf_test.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/test/integration/scheduler_perf/scheduler_perf_test.go b/test/integration/scheduler_perf/scheduler_perf_test.go index 3bca2bafdb0..ae6a3c7ffb9 100644 --- a/test/integration/scheduler_perf/scheduler_perf_test.go +++ b/test/integration/scheduler_perf/scheduler_perf_test.go @@ -134,16 +134,16 @@ type workload struct { // Name of the workload. Name string // Values of parameters used in the workloadTemplate. - Params Params + Params params } -type Params struct { +type params struct { params map[string]int // isUsed field records whether params is used or not. isUsed map[string]bool } -// UnmarshalJSON is a custom unmarshaler for Params. +// UnmarshalJSON is a custom unmarshaler for params. // // from(json): // { @@ -152,7 +152,7 @@ type Params struct { // } // // to: -// Params{ +// params{ // params: map[string]int{ // "intNodes": 500, // "initPods": 50, @@ -160,7 +160,7 @@ type Params struct { // isUsed: map[string]bool{}, // empty map // } // -func (p *Params) UnmarshalJSON(b []byte) error { +func (p *params) UnmarshalJSON(b []byte) error { aux := map[string]int{} if err := json.Unmarshal(b, &aux); err != nil { @@ -173,7 +173,7 @@ func (p *Params) UnmarshalJSON(b []byte) error { } // getParam gets param with key. -func (p Params) getParam(key string) (int, error) { +func (p params) getParam(key string) (int, error) { p.isUsed[key] = true param, ok := p.params[key] if ok { From 6bf6e424a1932aed8c801eda0ee2e061c1eb31db Mon Sep 17 00:00:00 2001 From: sanposhiho <44139130+sanposhiho@users.noreply.github.com> Date: Thu, 9 Sep 2021 07:11:36 +0900 Subject: [PATCH 3/4] =?UTF-8?q?Fix:=20rename=20getParams=E2=86=92get?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../scheduler_perf/scheduler_perf_test.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/test/integration/scheduler_perf/scheduler_perf_test.go b/test/integration/scheduler_perf/scheduler_perf_test.go index ae6a3c7ffb9..181e7a5bd0d 100644 --- a/test/integration/scheduler_perf/scheduler_perf_test.go +++ b/test/integration/scheduler_perf/scheduler_perf_test.go @@ -172,8 +172,8 @@ func (p *params) UnmarshalJSON(b []byte) error { return nil } -// getParam gets param with key. -func (p params) getParam(key string) (int, error) { +// get returns param. +func (p params) get(key string) (int, error) { p.isUsed[key] = true param, ok := p.params[key] if ok { @@ -284,7 +284,7 @@ func (*createNodesOp) collectsMetrics() bool { func (cno createNodesOp) patchParams(w *workload) (realOp, error) { if cno.CountParam != "" { var err error - cno.Count, err = w.Params.getParam(cno.CountParam[1:]) + cno.Count, err = w.Params.get(cno.CountParam[1:]) if err != nil { return nil, err } @@ -326,7 +326,7 @@ func (*createNamespacesOp) collectsMetrics() bool { func (cmo createNamespacesOp) patchParams(w *workload) (realOp, error) { if cmo.CountParam != "" { var err error - cmo.Count, err = w.Params.getParam(cmo.CountParam[1:]) + cmo.Count, err = w.Params.get(cmo.CountParam[1:]) if err != nil { return nil, err } @@ -386,7 +386,7 @@ func (cpo *createPodsOp) collectsMetrics() bool { func (cpo createPodsOp) patchParams(w *workload) (realOp, error) { if cpo.CountParam != "" { var err error - cpo.Count, err = w.Params.getParam(cpo.CountParam[1:]) + cpo.Count, err = w.Params.get(cpo.CountParam[1:]) if err != nil { return nil, err } @@ -428,7 +428,7 @@ func (cpso *createPodSetsOp) collectsMetrics() bool { func (cpso createPodSetsOp) patchParams(w *workload) (realOp, error) { if cpso.CountParam != "" { var err error - cpso.Count, err = w.Params.getParam(cpso.CountParam[1:]) + cpso.Count, err = w.Params.get(cpso.CountParam[1:]) if err != nil { return nil, err } From 1318f74609fb2a74182b8018134a0fdef7f5b14d Mon Sep 17 00:00:00 2001 From: sanposhiho <44139130+sanposhiho@users.noreply.github.com> Date: Thu, 9 Sep 2021 07:34:30 +0900 Subject: [PATCH 4/4] Fix: use Fatalf and list all unused params in one error --- test/integration/scheduler_perf/scheduler_perf_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/integration/scheduler_perf/scheduler_perf_test.go b/test/integration/scheduler_perf/scheduler_perf_test.go index 181e7a5bd0d..265823ae526 100644 --- a/test/integration/scheduler_perf/scheduler_perf_test.go +++ b/test/integration/scheduler_perf/scheduler_perf_test.go @@ -816,8 +816,8 @@ func runWorkload(b *testing.B, tc *testCase, w *workload) []DataItem { // check unused params and inform users unusedParams := w.unusedParams() - for _, p := range unusedParams { - b.Logf("the parameter %s is defined on workload %s, but it is unused. Please make sure there are no typos.", p, w.Name) + if len(unusedParams) != 0 { + b.Fatalf("the parameters %v are defined on workload %s, but unused.\nPlease make sure there are no typos.", unusedParams, w.Name) } // Some tests have unschedulable pods. Do not add an implicit barrier at the