Skip to content

Commit

Permalink
Merge pull request #158 from valaparthvi/pod-overrides
Browse files Browse the repository at this point in the history 
Add support for "pod-overrides" attribute
  • Loading branch information
@maysunfaisal
maysunfaisal authored Jan 5, 2023
2 parents 7beda08 + ae3d5d1 commit 4d107dd
Show file tree
Hide file tree
Showing 4 changed files with 515 additions and 17 deletions.
14 changes: 13 additions & 1 deletion pkg/devfile/generator/generators.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -186,8 +186,20 @@ func GetDeployment(devfileObj parser.DevfileObj, deployParams DeploymentParams)
Volumes: deployParams.Volumes,
}

globalAttributes, err := devfileObj.Data.GetAttributes()
if err != nil {
return nil, err
}
components, err := devfileObj.Data.GetDevfileContainerComponents(common.DevfileOptions{})
if err != nil {
return nil, err
}
podTemplateSpec, err := getPodTemplateSpec(globalAttributes, components, podTemplateSpecParams)
if err != nil {
return nil, err
}
deploySpecParams := deploymentSpecParams{
PodTemplateSpec: *getPodTemplateSpec(podTemplateSpecParams),
PodTemplateSpec: *podTemplateSpec,
PodSelectorLabels: deployParams.PodSelectorLabels,
Replicas: deployParams.Replicas,
}
Expand Down
99 changes: 88 additions & 11 deletions pkg/devfile/generator/generators_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,16 @@ package generator

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

"github.com/stretchr/testify/assert"
appsv1 "k8s.io/api/apps/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/intstr"
"k8s.io/utils/pointer"
"reflect"
"strings"
"testing"

v1 "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
"github.com/devfile/api/v2/pkg/attributes"
Expand Down Expand Up @@ -1076,7 +1078,9 @@ func TestGetDeployment(t *testing.T) {
name string
containerComponents []v1.Component
deploymentParams DeploymentParams
expected appsv1.Deployment
expected *appsv1.Deployment
attributes attributes.Attributes
wantErr bool
}{
{
// Currently dedicatedPod can only filter out annotations
Expand Down Expand Up @@ -1108,7 +1112,7 @@ func TestGetDeployment(t *testing.T) {
Containers: containers,
Replicas: pointer.Int32Ptr(1),
},
expected: appsv1.Deployment{
expected: &appsv1.Deployment{
ObjectMeta: objectMetaDedicatedPod,
Spec: appsv1.DeploymentSpec{
Strategy: appsv1.DeploymentStrategy{
Expand Down Expand Up @@ -1154,7 +1158,7 @@ func TestGetDeployment(t *testing.T) {
},
Containers: containers,
},
expected: appsv1.Deployment{
expected: &appsv1.Deployment{
ObjectMeta: objectMeta,
Spec: appsv1.DeploymentSpec{
Strategy: appsv1.DeploymentStrategy{
Expand All @@ -1172,6 +1176,78 @@ func TestGetDeployment(t *testing.T) {
},
},
},
{
name: "pod should have pod-overrides attribute",
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),
},
attributes: attributes.Attributes{
PodOverridesAttribute: apiext.JSON{Raw: []byte("{\"spec\": {\"serviceAccountName\": \"new-service-account\"}}")},
},
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,
ServiceAccountName: "new-service-account",
},
},
},
},
},
{
name: "pod has an invalid pod-overrides attribute that throws error",
containerComponents: []v1.Component{
testingutil.GenerateDummyContainerComponent("container2", nil, nil, nil, v1.Annotation{
Deployment: map[string]string{
"key2": "value2",
},
}, nil),
},
attributes: attributes.Attributes{
PodOverridesAttribute: apiext.JSON{Raw: []byte("{\"spec\": \"serviceAccountName\": \"new-service-account\"}}")},
},
deploymentParams: DeploymentParams{
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{
"preserved-key": "preserved-value",
},
},
Containers: containers,
},
expected: nil,
wantErr: trueBool,
},
}

for _, tt := range tests {
Expand All @@ -1186,18 +1262,19 @@ func TestGetDeployment(t *testing.T) {
},
}
// set up the mock data
mockGetComponents := mockDevfileData.EXPECT().GetComponents(options)
mockGetComponents.Return(tt.containerComponents, nil).AnyTimes()
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
if err != nil {
t.Errorf("TestGetDeployment(): unexpected error %v", err)
if !tt.wantErr == (err != nil) {
t.Errorf("TestGetDeployment(): unexpected error %v, wantErr %v", err, tt.wantErr)
}
assert.Equal(t, tt.expected, *deploy, "TestGetDeployment(): The two values should be the same.")
assert.Equal(t, tt.expected, deploy, "TestGetDeployment(): The two values should be the same.")

})
}
Expand Down
122 changes: 118 additions & 4 deletions pkg/devfile/generator/utils.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,27 +18,33 @@ package generator
import (
"encoding/json"
"fmt"
"github.com/hashicorp/go-multierror"
"path/filepath"
"reflect"
"strings"

v1 "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
"github.com/devfile/api/v2/pkg/attributes"

"github.com/devfile/library/v2/pkg/devfile/parser"
"github.com/devfile/library/v2/pkg/devfile/parser/data/v2/common"
"github.com/hashicorp/go-multierror"
buildv1 "github.com/openshift/api/build/v1"
routev1 "github.com/openshift/api/route/v1"
appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
extensionsv1 "k8s.io/api/extensions/v1beta1"
networkingv1 "k8s.io/api/networking/v1"
apiext "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/intstr"
"k8s.io/apimachinery/pkg/util/strategicpatch"
)

const ContainerOverridesAttribute = "container-overrides"
const (
ContainerOverridesAttribute = "container-overrides"
PodOverridesAttribute = "pod-overrides"
)

// convertEnvs converts environment variables from the devfile structure to kubernetes structure
func convertEnvs(vars []v1.EnvVar) []corev1.EnvVar {
Expand Down Expand Up @@ -235,7 +241,7 @@ type podTemplateSpecParams struct {
}

// getPodTemplateSpec gets a pod template spec that can be used to create a deployment spec
func getPodTemplateSpec(podTemplateSpecParams podTemplateSpecParams) *corev1.PodTemplateSpec {
func getPodTemplateSpec(globalAttributes attributes.Attributes, components []v1.Component, podTemplateSpecParams podTemplateSpecParams) (*corev1.PodTemplateSpec, error) {
podTemplateSpec := &corev1.PodTemplateSpec{
ObjectMeta: podTemplateSpecParams.ObjectMeta,
Spec: corev1.PodSpec{
Expand All @@ -245,7 +251,115 @@ func getPodTemplateSpec(podTemplateSpecParams podTemplateSpecParams) *corev1.Pod
},
}

return podTemplateSpec
if needsPodOverrides(globalAttributes, components) {
patchedPodTemplateSpec, err := applyPodOverrides(globalAttributes, components, podTemplateSpec)
if err != nil {
return nil, err
}
patchedPodTemplateSpec.ObjectMeta = podTemplateSpecParams.ObjectMeta
podTemplateSpec = patchedPodTemplateSpec
}

return podTemplateSpec, nil
}

// needsPodOverrides returns true if PodOverridesAttribute is present at Devfile or Container level attributes
func needsPodOverrides(globalAttributes attributes.Attributes, components []v1.Component) bool {
if globalAttributes.Exists(PodOverridesAttribute) {
return true
}
for _, component := range components {
if component.Attributes.Exists(PodOverridesAttribute) {
return true
}
}
return false
}

// applyPodOverrides returns a list of all the PodOverridesAttribute set at Devfile and Container level attributes
func applyPodOverrides(globalAttributes attributes.Attributes, components []v1.Component, podTemplateSpec *corev1.PodTemplateSpec) (*corev1.PodTemplateSpec, error) {
overrides, err := getPodOverrides(globalAttributes, components)
if err != nil {
return nil, err
}
// Workaround: the definition for corev1.PodSpec does not make containers optional, so even a nil list
// will be interpreted as "delete all containers" as the serialized patch will include "containers": null.
// To avoid this, save the original containers and reset them at the end.
originalContainers := podTemplateSpec.Spec.Containers
// Save fields we do not allow to be configured in pod-overrides
originalInitContainers := podTemplateSpec.Spec.InitContainers
originalVolumes := podTemplateSpec.Spec.Volumes

patchedTemplateBytes, err := json.Marshal(podTemplateSpec)
if err != nil {
return nil, fmt.Errorf("failed to marshal deployment to yaml: %w", err)
}
for _, override := range overrides {
patchedTemplateBytes, err = strategicpatch.StrategicMergePatch(patchedTemplateBytes, override.Raw, &corev1.PodTemplateSpec{})
if err != nil {
return nil, fmt.Errorf("error applying pod overrides: %w", err)
}
}
patchedPodTemplateSpec := corev1.PodTemplateSpec{}
if err := json.Unmarshal(patchedTemplateBytes, &patchedPodTemplateSpec); err != nil {
return nil, fmt.Errorf("error applying pod overrides: %w", err)
}
patchedPodTemplateSpec.Spec.Containers = originalContainers
patchedPodTemplateSpec.Spec.InitContainers = originalInitContainers
patchedPodTemplateSpec.Spec.Volumes = originalVolumes
return &patchedPodTemplateSpec, nil
}

// getPodOverrides returns PodTemplateSpecOverrides for every instance of the pod overrides attribute
// present in the DevWorkspace. The order of elements is
// 1. Pod overrides defined on Container components, in the order they appear in the DevWorkspace
// 2. Pod overrides defined in the global attributes field (.spec.template.attributes)
func getPodOverrides(globalAttributes attributes.Attributes, components []v1.Component) ([]apiext.JSON, error) {
var allOverrides []apiext.JSON

for _, component := range components {
if component.Attributes.Exists(PodOverridesAttribute) {
override := corev1.PodTemplateSpec{}
// Check format of pod-overrides to detect errors early
if err := component.Attributes.GetInto(PodOverridesAttribute, &override); err != nil {
return nil, fmt.Errorf("failed to parse %s attribute on component %s: %w", PodOverridesAttribute, component.Name, err)
}
// Do not allow overriding containers or volumes
if override.Spec.Containers != nil {
return nil, fmt.Errorf("cannot use %s to override pod containers (component %s)", PodOverridesAttribute, component.Name)
}
if override.Spec.InitContainers != nil {
return nil, fmt.Errorf("cannot use %s to override pod initContainers (component %s)", PodOverridesAttribute, component.Name)
}
if override.Spec.Volumes != nil {
return nil, fmt.Errorf("cannot use %s to override pod volumes (component %s)", PodOverridesAttribute, component.Name)
}
patchData := component.Attributes[PodOverridesAttribute]
allOverrides = append(allOverrides, patchData)
}
}

if globalAttributes.Exists(PodOverridesAttribute) {
override := corev1.PodTemplateSpec{}
err := globalAttributes.GetInto(PodOverridesAttribute, &override)
if err != nil {
return nil, fmt.Errorf("failed to parse %s attribute for pod: %w", PodOverridesAttribute, err)
}
// Do not allow overriding containers or volumes
if override.Spec.Containers != nil {
return nil, fmt.Errorf("cannot use %s to override pod containers", PodOverridesAttribute)
}
if override.Spec.InitContainers != nil {
return nil, fmt.Errorf("cannot use %s to override pod initContainers", PodOverridesAttribute)
}
if override.Spec.Volumes != nil {
return nil, fmt.Errorf("cannot use %s to override pod volumes", PodOverridesAttribute)
}
patchData := globalAttributes[PodOverridesAttribute]
allOverrides = append(allOverrides, patchData)
}

return allOverrides, nil
}

// deploymentSpecParams is a struct that contains the required data to create a deployment spec object
Expand Down
Loading

0 comments on commit 4d107dd

Please sign in to comment.