Skip to content

Commit

Permalink
Fix GetDeployment function when Devfile SchemaVersion is less than 2.…
Browse files Browse the repository at this point in the history 
…1.0 (#161)

* Fix GetDeployment function when Devfile SchemaVersion is less than 2.1.0

Signed-off-by: Parthvi Vala <pvala@redhat.com>

* Add test coverage

Signed-off-by: Parthvi Vala <pvala@redhat.com>

* Remove error check for GetAttributes

Signed-off-by: Parthvi Vala <pvala@redhat.com>

Signed-off-by: Parthvi Vala <pvala@redhat.com>
  • Loading branch information
@valaparthvi
valaparthvi authored Jan 18, 2023
1 parent 4d107dd commit 0d04f79
Show file tree
Hide file tree
Showing 4 changed files with 116 additions and 22 deletions.
13 changes: 9 additions & 4 deletions pkg/devfile/generator/generators.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ package generator
import (
"fmt"

"github.com/devfile/api/v2/pkg/attributes"
"github.com/devfile/library/v2/pkg/devfile/parser/data"

v1 "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
"github.com/devfile/library/v2/pkg/devfile/parser"
"github.com/devfile/library/v2/pkg/devfile/parser/data/v2/common"
Expand Down Expand Up @@ -185,10 +188,12 @@ func GetDeployment(devfileObj parser.DevfileObj, deployParams DeploymentParams)
Containers: deployParams.Containers,
Volumes: deployParams.Volumes,
}

globalAttributes, err := devfileObj.Data.GetAttributes()
if err != nil {
return nil, err
var globalAttributes attributes.Attributes
// attributes is not supported in versions less than 2.1.0, so we skip it
if devfileObj.Data.GetSchemaVersion() > string(data.APISchemaVersion200) {
// the only time GetAttributes will return an error is if DevfileSchemaVersion is 2.0.0, a case we've already covered;
// so we'll skip checking for error here
globalAttributes, _ = devfileObj.Data.GetAttributes()
}
components, err := devfileObj.Data.GetDevfileContainerComponents(common.DevfileOptions{})
if err != nil {
Expand Down
101 changes: 87 additions & 14 deletions pkg/devfile/generator/generators_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,12 @@ package generator

import (
"fmt"
apiext "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
"reflect"
"strings"
"testing"

apiext "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"

"github.com/stretchr/testify/assert"
appsv1 "k8s.io/api/apps/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -1081,6 +1082,7 @@ func TestGetDeployment(t *testing.T) {
expected *appsv1.Deployment
attributes attributes.Attributes
wantErr bool
devObj func(ctrl *gomock.Controller, containerComponents []v1.Component) parser.DevfileObj
}{
{
// Currently dedicatedPod can only filter out annotations
Expand Down Expand Up @@ -1248,26 +1250,97 @@ func TestGetDeployment(t *testing.T) {
expected: nil,
wantErr: trueBool,
},
{
name: "skip getting global attributes for SchemaVersion less than 2.1.0",
containerComponents: []v1.Component{
testingutil.GenerateDummyContainerComponent("container1", nil, []v1.Endpoint{
{
Name: "http-8080",
TargetPort: 8080,
},
}, nil, v1.Annotation{
Deployment: map[string]string{
"key1": "value1",
},
}, nil),
testingutil.GenerateDummyContainerComponent("container2", nil, nil, nil, v1.Annotation{
Deployment: map[string]string{
"key2": "value2",
},
}, nil),
},
deploymentParams: DeploymentParams{
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{
"preserved-key": "preserved-value",
},
},
Containers: containers,
},
expected: &appsv1.Deployment{
ObjectMeta: objectMeta,
Spec: appsv1.DeploymentSpec{
Strategy: appsv1.DeploymentStrategy{
Type: appsv1.RecreateDeploymentStrategyType,
},
Selector: &metav1.LabelSelector{
MatchLabels: nil,
},
Template: corev1.PodTemplateSpec{
ObjectMeta: objectMeta,
Spec: corev1.PodSpec{
Containers: containers,
},
},
},
},
attributes: nil,
wantErr: false,
devObj: func(ctrl *gomock.Controller, containerComponents []v1.Component) parser.DevfileObj {
mockDevfileData := data.NewMockDevfileData(ctrl)

options := common.DevfileOptions{
ComponentOptions: common.ComponentOptions{
ComponentType: v1.ContainerComponentType,
},
}
// set up the mock data
mockDevfileData.EXPECT().GetSchemaVersion().Return("2.0.0")
mockDevfileData.EXPECT().GetDevfileContainerComponents(common.DevfileOptions{}).Return(containerComponents, nil).AnyTimes()
mockDevfileData.EXPECT().GetComponents(options).Return(containerComponents, nil).AnyTimes()

devObj := parser.DevfileObj{
Data: mockDevfileData,
}
return devObj
},
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()
mockDevfileData := data.NewMockDevfileData(ctrl)

options := common.DevfileOptions{
ComponentOptions: common.ComponentOptions{
ComponentType: v1.ContainerComponentType,
},
}
// set up the mock data
mockDevfileData.EXPECT().GetAttributes().Return(tt.attributes, nil).AnyTimes()
mockDevfileData.EXPECT().GetDevfileContainerComponents(common.DevfileOptions{}).Return(tt.containerComponents, nil).AnyTimes()
mockDevfileData.EXPECT().GetComponents(options).Return(tt.containerComponents, nil).AnyTimes()
var devObj parser.DevfileObj
if tt.devObj != nil {
devObj = tt.devObj(ctrl, tt.containerComponents)
} else {
mockDevfileData := data.NewMockDevfileData(ctrl)

devObj := parser.DevfileObj{
Data: mockDevfileData,
options := common.DevfileOptions{
ComponentOptions: common.ComponentOptions{
ComponentType: v1.ContainerComponentType,
},
}
// set up the mock data
mockDevfileData.EXPECT().GetSchemaVersion().Return("2.1.0")
mockDevfileData.EXPECT().GetAttributes().Return(tt.attributes, nil).AnyTimes()
mockDevfileData.EXPECT().GetDevfileContainerComponents(common.DevfileOptions{}).Return(tt.containerComponents, nil).AnyTimes()
mockDevfileData.EXPECT().GetComponents(options).Return(tt.containerComponents, nil).AnyTimes()

devObj = parser.DevfileObj{
Data: mockDevfileData,
}
}
deploy, err := GetDeployment(devObj, tt.deploymentParams)
// Checks for unexpected error cases
Expand Down
3 changes: 1 addition & 2 deletions pkg/devfile/generator/utils.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -250,8 +250,7 @@ func getPodTemplateSpec(globalAttributes attributes.Attributes, components []v1.
Volumes: podTemplateSpecParams.Volumes,
},
}

if needsPodOverrides(globalAttributes, components) {
if len(globalAttributes) != 0 && needsPodOverrides(globalAttributes, components) {
patchedPodTemplateSpec, err := applyPodOverrides(globalAttributes, components, podTemplateSpec)
if err != nil {
return nil, err
Expand Down
21 changes: 19 additions & 2 deletions pkg/devfile/generator/utils_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package generator

import (
"fmt"

"github.com/hashicorp/go-multierror"
"github.com/stretchr/testify/assert"
"k8s.io/utils/pointer"
Expand All @@ -37,6 +38,7 @@ import (
v1 "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"

corev1 "k8s.io/api/core/v1"
apiext "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -640,15 +642,27 @@ func TestGetPodTemplateSpec(t *testing.T) {
namespace string
serviceAccount string
labels map[string]string
attributes attributes.Attributes
}{
{
podName: "podSpecTest",
namespace: "default",
labels: map[string]string{
"app": "app",
"component": "frontend",
},
},
{
podName: "podSpecTest",
namespace: "default",
serviceAccount: "default",
serviceAccount: "new-service-account",
labels: map[string]string{
"app": "app",
"component": "frontend",
},
attributes: attributes.Attributes{
PodOverridesAttribute: apiext.JSON{Raw: []byte("{\"spec\": {\"serviceAccountName\": \"new-service-account\"}}")},
},
},
}

Expand All @@ -663,7 +677,7 @@ func TestGetPodTemplateSpec(t *testing.T) {
InitContainers: container,
}

podTemplateSpec, err := getPodTemplateSpec(nil, nil, podTemplateSpecParams)
podTemplateSpec, err := getPodTemplateSpec(tt.attributes, nil, podTemplateSpecParams)
if err != nil {
t.Errorf("TestGetPodTemplateSpec() error: %s", err.Error())
}
Expand All @@ -674,6 +688,9 @@ func TestGetPodTemplateSpec(t *testing.T) {
if podTemplateSpec.Namespace != tt.namespace {
t.Errorf("TestGetPodTemplateSpec() error: expected namespace %s, actual %s", tt.namespace, podTemplateSpec.Namespace)
}
if tt.serviceAccount != "" && podTemplateSpec.Spec.ServiceAccountName != tt.serviceAccount {
t.Errorf("TestGetPodTemplateSpec() error: expected serviceAccountName %s, actual %s", tt.serviceAccount, podTemplateSpec.Spec.ServiceAccountName)
}
if !hasVolumeWithName("vol1", podTemplateSpec.Spec.Volumes) {
t.Errorf("TestGetPodTemplateSpec() error: volume with name: %s not found", "vol1")
}
Expand Down

0 comments on commit 0d04f79

Please sign in to comment.