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.

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

Implementation of KEP-30: Immutable Parameters #1575

Merged
merged 13 commits into from
Jul 20, 2020
4 changes: 4 additions & 0 deletions config/crds/kudo.dev_operatorversions.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,10 @@ spec:
displayName:
description: DisplayName can be used by UIs.
type: string
immutable:
description: Specifies if the parameter can be changed after the
initial installation of the operator
type: boolean
name:
description: "Name is the string that should be used in the template
file for example, if `name: COUNT` then using the variable in
Expand Down
42 changes: 0 additions & 42 deletions pkg/apis/kudo/v1beta1/instance_types_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -222,48 +222,6 @@ func GetParamDefinitions(params map[string]string, ov *OperatorVersion) ([]Param
return defs, nil
}

// ParameterDiff returns map containing all parameters that were removed or changed between old and new
func ParameterDiff(old, new map[string]string) map[string]string {
diff := make(map[string]string)

for key, val := range old {
// If a parameter was removed in the new spec
if _, ok := new[key]; !ok {
diff[key] = val
}
}

for key, val := range new {
// If new spec parameter was added or changed
if v, ok := old[key]; !ok || v != val {
diff[key] = val
}
}

return diff
}

// RichParameterDiff compares new and old map and returns two maps: first containing all changed/added
// and second all removed parameters.
func RichParameterDiff(old, new map[string]string) (changed, removed map[string]string) {
changed, removed = make(map[string]string), make(map[string]string)

for key, val := range old {
// If a parameter was removed in the new spec
if _, ok := new[key]; !ok {
removed[key] = val
}
}

for key, val := range new {
// If new spec parameter was added or changed
if v, ok := old[key]; !ok || v != val {
changed[key] = val
}
}
return
}

func CleanupPlanExists(ov *OperatorVersion) bool { return PlanExists(CleanupPlanName, ov) }

func PlanExists(plan string, ov *OperatorVersion) bool {
Expand Down
29 changes: 0 additions & 29 deletions pkg/apis/kudo/v1beta1/operatorversion_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,35 +82,6 @@ const (
MapValueType ParameterType = "map"
)

// Parameter captures the variability of an OperatorVersion being instantiated in an instance.
type Parameter struct {
// DisplayName can be used by UIs.
DisplayName string `json:"displayName,omitempty"`

// Name is the string that should be used in the template file for example,
// if `name: COUNT` then using the variable in a spec like:
//
// spec:
// replicas: {{ .Params.COUNT }}
Name string `json:"name,omitempty"`

// Description captures a longer description of how the parameter will be used.
Description string `json:"description,omitempty"`

// Required specifies if the parameter is required to be provided by all instances, or whether a default can suffice.
Required *bool `json:"required,omitempty"`

// Default is a default value if no parameter is provided by the instance.
Default *string `json:"default,omitempty"`

// Trigger identifies the plan that gets executed when this parameter changes in the Instance object.
// Default is `update` if a plan with that name exists, otherwise it's `deploy`.
Trigger string `json:"trigger,omitempty"`

// Type specifies the value type. Defaults to `string`.
Type ParameterType `json:"value-type,omitempty"`
}

// Phase specifies a list of steps that contain Kubernetes objects.
type Phase struct {
// +optional
Expand Down
33 changes: 33 additions & 0 deletions pkg/apis/kudo/v1beta1/parameter_types.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package v1beta1

// Parameter captures the variability of an OperatorVersion being instantiated in an instance.
type Parameter struct {
// DisplayName can be used by UIs.
DisplayName string `json:"displayName,omitempty"`

// Name is the string that should be used in the template file for example,
// if `name: COUNT` then using the variable in a spec like:
//
// spec:
// replicas: {{ .Params.COUNT }}
Name string `json:"name,omitempty"`

// Description captures a longer description of how the parameter will be used.
Description string `json:"description,omitempty"`

// Required specifies if the parameter is required to be provided by all instances, or whether a default can suffice.
Required *bool `json:"required,omitempty"`

// Default is a default value if no parameter is provided by the instance.
Default *string `json:"default,omitempty"`

// Trigger identifies the plan that gets executed when this parameter changes in the Instance object.
// Default is `update` if a plan with that name exists, otherwise it's `deploy`.
Trigger string `json:"trigger,omitempty"`

// Type specifies the value type. Defaults to `string`.
Type ParameterType `json:"value-type,omitempty"`

// Specifies if the parameter can be changed after the initial installation of the operator
Immutable *bool `json:"immutable,omitempty"`
}
83 changes: 83 additions & 0 deletions pkg/apis/kudo/v1beta1/parameter_types_helpers.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
package v1beta1

func (p *Parameter) IsImmutable() bool {
return p.Immutable != nil && *p.Immutable
}

func (p *Parameter) IsRequired() bool {
return p.Required != nil && *p.Required
}

func (p *Parameter) HasDefault() bool {
return p.Default != nil
}

// GetChangedParamDefs returns a list of parameters from ov2 that changed based on the given compare function between ov1 and ov2
func GetChangedParamDefs(p1, p2 []Parameter, isEqual func(p1, p2 Parameter) bool) []Parameter {
changedParams := []Parameter{}

for _, p1 := range p1 {
for _, p2 := range p2 {
if p1.Name == p2.Name {
if !isEqual(p1, p2) {
changedParams = append(changedParams, p2)
}
}
}
}

return changedParams
}

// GetAddedParameters returns a list of parameters that are in oldOv but not in newOv
func GetRemovedParamDefs(old, new []Parameter) []Parameter {
return GetAddedParameters(new, old)
}

// GetAddedParameters returns a list of parameters that are in newOv but not in oldOv
func GetAddedParameters(old, new []Parameter) []Parameter {
addedParams := []Parameter{}

NewParams:
for _, newParam := range new {
for _, oldParam := range old {
if newParam.Name == oldParam.Name {
continue NewParams
}
}
addedParams = append(addedParams, newParam)
}
return addedParams
}

// ParameterDiff returns map containing all parameters that were removed or changed between old and new
func ParameterDiff(old, new map[string]string) map[string]string {
changed, removed := RichParameterDiff(old, new)

// Join both maps
for key, val := range removed {
changed[key] = val
}
return changed
}

// RichParameterDiff compares new and old map and returns two maps: first containing all changed/added
// and second all removed parameters.
func RichParameterDiff(old, new map[string]string) (changed, removed map[string]string) {
changed, removed = make(map[string]string), make(map[string]string)

for key, val := range old {
// If a parameter was removed in the new spec
if _, ok := new[key]; !ok {
removed[key] = val
}
}

for key, val := range new {
// If new spec parameter was added or changed
if v, ok := old[key]; !ok || v != val {
changed[key] = val
}
}
return
}
139 changes: 139 additions & 0 deletions pkg/apis/kudo/v1beta1/parameter_types_helpers_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,139 @@
/*

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package v1beta1

import (
"testing"

"github.com/stretchr/testify/assert"
)

func TestGetChangedParamDefs(t *testing.T) {
bTrue := true
cmpImmutable := func(p1, p2 Parameter) bool {
return p1.Immutable == p2.Immutable
}

tests := []struct {
name string
old []Parameter
new []Parameter
want []Parameter
cmp func(p1, p2 Parameter) bool
}{
{
name: "no changes with empty",
old: []Parameter{},
new: []Parameter{},
want: []Parameter{},
cmp: cmpImmutable,
},
{
name: "no changes with content",
old: []Parameter{{Name: "p"}},
new: []Parameter{{Name: "p"}},
want: []Parameter{},
cmp: cmpImmutable,
},
{
name: "changed attribute",
old: []Parameter{{Name: "p"}},
new: []Parameter{{Name: "p", Immutable: &bTrue}},
want: []Parameter{{Name: "p", Immutable: &bTrue}},
cmp: cmpImmutable,
},
{
name: "changed other attribute does not register",
old: []Parameter{{Name: "p"}},
new: []Parameter{{Name: "p", Required: &bTrue}},
want: []Parameter{},
cmp: cmpImmutable,
},
{
name: "added and removed params do not show up",
old: []Parameter{{Name: "p"}},
new: []Parameter{{Name: "p1"}},
want: []Parameter{},
cmp: cmpImmutable,
},
}

for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
changedDefs := GetChangedParamDefs(tt.old, tt.new, tt.cmp)
assert.Equal(t, tt.want, changedDefs)
})
}
}

func TestGetAddedRemovedParamDefs(t *testing.T) {
bTrue := true

tests := []struct {
name string
old []Parameter
new []Parameter
wantAdded []Parameter
wantRemoved []Parameter
}{
{
name: "no result when empty",
old: []Parameter{},
new: []Parameter{},
wantAdded: []Parameter{},
wantRemoved: []Parameter{},
},
{
name: "no result when no changes",
old: []Parameter{{Name: "p"}},
new: []Parameter{{Name: "p"}},
wantAdded: []Parameter{},
wantRemoved: []Parameter{},
},
{
name: "added/removed param show up in correct direct",
old: []Parameter{{Name: "p"}},
new: []Parameter{{Name: "p"}, {Name: "p1"}},
wantAdded: []Parameter{{Name: "p1"}},
wantRemoved: []Parameter{},
},
{
name: "added param does not show up in other direction",
old: []Parameter{{Name: "p"}, {Name: "p1"}},
new: []Parameter{{Name: "p"}},
wantAdded: []Parameter{},
wantRemoved: []Parameter{{Name: "p1"}},
},
{
name: "changed attribute does not register",
old: []Parameter{{Name: "p"}},
new: []Parameter{{Name: "p", Required: &bTrue}},
wantAdded: []Parameter{},
wantRemoved: []Parameter{},
},
}

for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
addedDefs := GetAddedParameters(tt.old, tt.new)
removedDefs := GetRemovedParamDefs(tt.old, tt.new)
assert.Equal(t, tt.wantAdded, addedDefs)
assert.Equal(t, tt.wantRemoved, removedDefs)
})
}
}
5 changes: 5 additions & 0 deletions pkg/apis/kudo/v1beta1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions pkg/kudoctl/cmd/generate/testdata/parameter.golden
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,12 @@ apiVersion: kudo.dev/v1beta1
parameters:
- default: 1Gi
description: Amount of memory to provide to Zookeeper pods
immutable: false
name: memory
required: true
- default: "0.25"
description: Amount of cpu to provide to Zookeeper pods
immutable: false
name: cpus
required: true
- default: Bar
Expand Down
Loading