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.

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

Jump to bottom

feat: check for incompatible flavors #146

Merged
merged 3 commits into from
Jan 24, 2024
Merged

feat: check for incompatible flavors #146

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
366ee4a
feat: check for incompatible flavors
crgisch Jan 23, 2024
686c413
chore: update crd
crgisch Jan 24, 2024
c28e45d
refactor: review suggestions
crgisch Jan 24, 2024
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
4 changes: 4 additions & 0 deletions api/v1alpha1/rpaasflavor_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,10 @@ type RpaasFlavorSpec struct {
// CreationOnly defines if the flavor could be used only in the moment of creation of instance
// +optional
CreationOnly bool `json:"creationOnly,omitempty"`

// IncompatibleFlavors defines which other flavors cannot be used with this flavor
// +optional
IncompatibleFlavors []string `json:"incompatibleFlavors,omitempty"`
}

// +kubebuilder:object:root=true
Expand Down
17 changes: 17 additions & 0 deletions internal/pkg/rpaas/k8s.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1289,6 +1289,11 @@ func (m *k8sRpaasManager) validateFlavors(ctx context.Context, instance *v1alpha
if flavorObj.Spec.CreationOnly && !isCreation {
return &ValidationError{Msg: fmt.Sprintf("flavor %q can used only in the creation of instance", f)}
}

incompatibleFlavor := checkIncompatibleFlavors(flavorObj.Spec.IncompatibleFlavors, flavors)
Copy link
Member

@morpheu morpheu Jan 24, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I think is just cosmetic, but would be nice bring error message inside checkIncompatibleFlavors and throw it instead a string to be more golang way to handle error

if incompatibleFlavor != "" {
return &ValidationError{Msg: fmt.Sprintf("flavor %q is incompatible with %q flavor", f, incompatibleFlavor)}
}
}

for _, f := range removed {
Expand Down Expand Up @@ -1321,6 +1326,18 @@ func diffFlavors(existing, updated []string) (added, removed []string) {
return
}

func checkIncompatibleFlavors(incompatibleFlavors, allFlavors []string) string {
if len(incompatibleFlavors) > 0 {
for _, incompatibleFlavor := range incompatibleFlavors {
if contains(allFlavors, incompatibleFlavor) {
return incompatibleFlavor
}
}
}

return ""
}

func isBlockTypeAllowed(bt v1alpha1.BlockType) bool {
allowedBlockTypes := map[v1alpha1.BlockType]bool{
v1alpha1.BlockTypeRoot: true,
Expand Down
97 changes: 94 additions & 3 deletions internal/pkg/rpaas/k8s_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2573,6 +2573,26 @@ func Test_k8sRpaasManager_CreateInstance(t *testing.T) {
Description: "aaaaa",
},
},
&v1alpha1.RpaasFlavor{
ObjectMeta: metav1.ObjectMeta{
Name: "vanilla",
Namespace: getServiceName(),
},
Spec: v1alpha1.RpaasFlavorSpec{
Description: "aaaaa",
IncompatibleFlavors: []string{"strawberry", "chocolate"},
},
},
&v1alpha1.RpaasFlavor{
ObjectMeta: metav1.ObjectMeta{
Name: "chocolate",
Namespace: getServiceName(),
},
Spec: v1alpha1.RpaasFlavorSpec{
Description: "aaaaa",
IncompatibleFlavors: []string{"mint-chocolate", "pecan"},
},
},
}
one := int32(1)
tests := []struct {
Expand Down Expand Up @@ -2618,6 +2638,11 @@ func Test_k8sRpaasManager_CreateInstance(t *testing.T) {
args: CreateArgs{Name: "r1", Team: "t1", Parameters: map[string]interface{}{"flavors": map[string]interface{}{"0": "strawberry", "1": "strawberry"}}},
expectedError: `flavor "strawberry" only can be set once`,
},
{
name: "incompatible flavors",
args: CreateArgs{Name: "r1", Team: "t1", Parameters: map[string]interface{}{"flavors": map[string]interface{}{"0": "strawberry", "1": "vanilla"}}},
expectedError: `flavor "vanilla" is incompatible with "strawberry" flavor`,
},
{
name: "instance already exists",
args: CreateArgs{Name: "r0", Team: "t2"},
Expand Down Expand Up @@ -2951,7 +2976,7 @@ func Test_k8sRpaasManager_CreateInstance(t *testing.T) {
},
{
name: "with flavor",
args: CreateArgs{Name: "r1", Team: "t1", Parameters: map[string]interface{}{"flavors": map[string]interface{}{"0": "strawberry"}}},
args: CreateArgs{Name: "r1", Team: "t1", Parameters: map[string]interface{}{"flavors": map[string]interface{}{"0": "strawberry", "1": "chocolate"}}},
expected: v1alpha1.RpaasInstance{
TypeMeta: metav1.TypeMeta{
Kind: "RpaasInstance",
Expand All @@ -2977,7 +3002,7 @@ func Test_k8sRpaasManager_CreateInstance(t *testing.T) {
Spec: v1alpha1.RpaasInstanceSpec{
Replicas: &one,
PlanName: "plan1",
Flavors: []string{"strawberry"},
Flavors: []string{"strawberry", "chocolate"},
Service: &nginxv1alpha1.NginxService{
Labels: map[string]string{
"rpaas.extensions.tsuru.io/service-name": "rpaasv2",
Expand Down Expand Up @@ -3334,7 +3359,39 @@ func Test_k8sRpaasManager_UpdateInstance(t *testing.T) {
},
}

resources := []runtime.Object{instance1, instance2, plan1, plan2, creationOnlyFlavor}
flavor1 := &v1alpha1.RpaasFlavor{
ObjectMeta: metav1.ObjectMeta{
Name: "flavor1",
Namespace: getServiceName(),
},
Spec: v1alpha1.RpaasFlavorSpec{
Description: "aaaaa",
},
}

flavor2 := &v1alpha1.RpaasFlavor{
ObjectMeta: metav1.ObjectMeta{
Name: "flavor2",
Namespace: getServiceName(),
},
Spec: v1alpha1.RpaasFlavorSpec{
Description: "aaaaa",
IncompatibleFlavors: []string{"flavor1", "flavor3"},
},
}

flavor3 := &v1alpha1.RpaasFlavor{
ObjectMeta: metav1.ObjectMeta{
Name: "flavor3",
Namespace: getServiceName(),
},
Spec: v1alpha1.RpaasFlavorSpec{
Description: "aaaaa",
IncompatibleFlavors: []string{"flavor2", "flavor-non-existent"},
},
}

resources := []runtime.Object{instance1, instance2, plan1, plan2, creationOnlyFlavor, flavor1, flavor2, flavor3}

tests := []struct {
name string
Expand Down Expand Up @@ -3387,6 +3444,39 @@ func Test_k8sRpaasManager_UpdateInstance(t *testing.T) {
assert.Equal(t, `cannot unset flavor "feature-create-only" as it is a creation only flavor`, err.Error())
},
},
{
name: "when added flavors are compatible",
instance: "instance1",
args: UpdateInstanceArgs{
Description: "Another description",
Plan: "plan1",
Team: "team-one",
Parameters: map[string]interface{}{
"lb-name": "my-instance.example",
"flavors": "flavor1,flavor3",
},
},
assertion: func(t *testing.T, err error, instance *v1alpha1.RpaasInstance) {
require.NoError(t, err)
},
},
{
name: "when tries to add incompatible flavors",
instance: "instance1",
args: UpdateInstanceArgs{
Description: "Another description",
Plan: "plan1",
Team: "team-one",
Parameters: map[string]interface{}{
"lb-name": "my-instance.example",
"flavors": "flavor1,flavor2",
},
},
assertion: func(t *testing.T, err error, instance *v1alpha1.RpaasInstance) {
require.Error(t, err)
assert.Equal(t, `flavor "flavor2" is incompatible with "flavor1" flavor`, err.Error())
},
},
{
name: "when tries to set an invalid annotation format",
instance: "instance1",
Expand Down Expand Up @@ -3415,6 +3505,7 @@ func Test_k8sRpaasManager_UpdateInstance(t *testing.T) {
Parameters: map[string]interface{}{
"lb-name": "my-instance.example",
"annotations": "{\"my-custom-annotation\": \"my-value\"}",
"flavors": "flavor3",
},
},
assertion: func(t *testing.T, err error, instance *v1alpha1.RpaasInstance) {
Expand Down
Loading