Skip to content

Commit

Permalink
Scoped recommended labels to apply to Pod and Service, added unit-tes…
Browse files Browse the repository at this point in the history
…ts and integration-tests for both Application and SKIPJob
  • Loading branch information
SnorreSelmer committed Sep 4, 2024
1 parent d8e615a commit a4bba73
Show file tree
Hide file tree
Showing 15 changed files with 130 additions and 27 deletions.
1 change: 0 additions & 1 deletion api/v1alpha1/application_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -418,7 +418,6 @@ func (a *Application) SetStatus(status SkiperatorStatus) {
func (a *Application) GetDefaultLabels() map[string]string {
return map[string]string{
"app.kubernetes.io/name": a.Name,
"app.kubernetes.io/version": getVersionLabel(a.Spec.Image),
"app.kubernetes.io/managed-by": "skiperator",
"skiperator.kartverket.no/controller": "application",
"application.skiperator.no/app": a.Name,
Expand Down
1 change: 0 additions & 1 deletion api/v1alpha1/skipjob_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,6 @@ func (skipJob *SKIPJob) FillDefaultStatus() {
func (skipJob *SKIPJob) GetDefaultLabels() map[string]string {
return map[string]string{
"app.kubernetes.io/name": skipJob.Name,
"app.kubernetes.io/version": getVersionLabel(skipJob.Spec.Container.Image),
"app.kubernetes.io/managed-by": "skiperator",
"skiperator.kartverket.no/controller": "skipjob",
// Used by hahaha to know that the Pod should be watched for killing sidecars
Expand Down
23 changes: 0 additions & 23 deletions api/v1alpha1/skipobj_interfaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"fmt"
"github.com/kartverket/skiperator/api/v1alpha1/podtypes"
"sigs.k8s.io/controller-runtime/pkg/client"
"strings"
)

type SKIPObject interface {
Expand All @@ -22,25 +21,3 @@ type CommonSpec struct {
AccessPolicy *podtypes.AccessPolicy
GCP *podtypes.GCP
}

func getVersionLabel(imageVersionString string) string {
parts := strings.Split(imageVersionString, ":")

// Implicitly assume version "latest" if no version is specified
if len(parts) < 2 {
return "latest"
}

versionPart := parts[1]

// Remove "@sha256" from version text if version includes a hash
if strings.Contains(versionPart, "@") {
versionPart = strings.Split(versionPart, "@")[0]
}

// Add build number to version if it is specified
if len(parts) > 2 {
return versionPart + "+" + parts[2]
}
return versionPart
}
1 change: 1 addition & 0 deletions pkg/resourcegenerator/deployment/deployment.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ func Generate(r reconciliation.Reconciliation) error {
} else {
podTemplateLabels = util.GetPodAppSelector(application.Name)
}
podTemplateLabels["app.kubernetes.io/version"] = resourceutils.GetImageVersion(application.Spec.Image)

// Add annotations to pod template, safe-to-evict added due to issues
// with cluster-autoscaler and unable to evict pods with local volumes
Expand Down
8 changes: 7 additions & 1 deletion pkg/resourcegenerator/job/job.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"github.com/kartverket/skiperator/pkg/reconciliation"
"github.com/kartverket/skiperator/pkg/resourcegenerator/gcp"
"github.com/kartverket/skiperator/pkg/resourcegenerator/pod"
"github.com/kartverket/skiperator/pkg/resourcegenerator/resourceutils"
"github.com/kartverket/skiperator/pkg/resourcegenerator/volume"
"github.com/kartverket/skiperator/pkg/util"
batchv1 "k8s.io/api/batch/v1"
Expand All @@ -27,7 +28,10 @@ func Generate(r reconciliation.Reconciliation) error {
meta := metav1.ObjectMeta{
Namespace: skipJob.Namespace,
Name: skipJob.Name,
Labels: map[string]string{"app": skipJob.KindPostFixedName()},
Labels: map[string]string{
"app": skipJob.KindPostFixedName(),
"app.kubernetes.io/version": resourceutils.GetImageVersion(skipJob.Spec.Container.Image),
},
}
job := batchv1.Job{ObjectMeta: meta}
cronJob := batchv1.CronJob{ObjectMeta: meta}
Expand Down Expand Up @@ -72,6 +76,7 @@ func getCronJobSpec(skipJob *skiperatorv1alpha1.SKIPJob, selector *metav1.LabelS
// it's not a default label, maybe it could be?
// used for selecting workloads by netpols, grafana etc
spec.JobTemplate.Labels["app"] = skipJob.KindPostFixedName()
spec.JobTemplate.Labels["app.kubernetes.io/version"] = resourceutils.GetImageVersion(skipJob.Spec.Container.Image)

return spec
}
Expand Down Expand Up @@ -127,6 +132,7 @@ func getJobSpec(skipJob *skiperatorv1alpha1.SKIPJob, selector *metav1.LabelSelec
// it's not a default label, maybe it could be?
// used for selecting workloads by netpols, grafana etc
jobSpec.Template.Labels["app"] = skipJob.KindPostFixedName()
jobSpec.Template.Labels["app.kubernetes.io/version"] = resourceutils.GetImageVersion(skipJob.Spec.Container.Image)

return jobSpec
}
23 changes: 23 additions & 0 deletions pkg/resourcegenerator/resourceutils/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package resourceutils
import (
skiperatorv1alpha1 "github.com/kartverket/skiperator/api/v1alpha1"
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
"strings"
)

func ShouldScaleToZero(jsonReplicas *apiextensionsv1.JSON) bool {
Expand All @@ -16,3 +17,25 @@ func ShouldScaleToZero(jsonReplicas *apiextensionsv1.JSON) bool {
}
return false
}

func GetImageVersion(imageVersionString string) string {
parts := strings.Split(imageVersionString, ":")

// Implicitly assume version "latest" if no version is specified
if len(parts) < 2 {
return "latest"
}

versionPart := parts[1]

// Remove "@sha256" from version text if version includes a hash
if strings.Contains(versionPart, "@") {
versionPart = strings.Split(versionPart, "@")[0]
}

// Add build number to version if it is specified
if len(parts) > 2 {
return versionPart + "+" + parts[2]
}
return versionPart
}
47 changes: 47 additions & 0 deletions pkg/resourcegenerator/resourceutils/helpers_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
package resourceutils

import (
"github.com/stretchr/testify/assert"
"testing"
)

func TestGetImageVersionNoTag(t *testing.T) {
imageString := "image"
expectedImageString := "latest"

actualImageString := GetImageVersion(imageString)

assert.Equal(t, expectedImageString, actualImageString)
}

func TestGetImageVersionLatestTag(t *testing.T) {
imageString := "image:latest"
expectedImageString := "latest"

actualImageString := GetImageVersion(imageString)

assert.Equal(t, expectedImageString, actualImageString)
}

func TestGetImageVersionVersionTag(t *testing.T) {
versionImageString := "image:1.2.3"
devImageString := "image:1.2.3-dev-123abc"
expectedVersionImageString := "1.2.3"
expectedDevImageString := "1.2.3-dev-123abc"

actualVersionImageString := GetImageVersion(versionImageString)
actualDevImageString := GetImageVersion(devImageString)

assert.Equal(t, expectedVersionImageString, actualVersionImageString)
assert.Equal(t, expectedDevImageString, actualDevImageString)

}

func TestGetImageVersionShaTag(t *testing.T) {
imageString := "ghcr.io/org/repo@sha256:54d7ea8b48d0e7569766e0e10b9e38da778a5f65d764168dd7db76a37d6b8"
expectedImageString := "54d7ea8b48d0e7569766e0e10b9e38da778a5f65d764168dd7db76a37d6b8"

actualImageString := GetImageVersion(imageString)

assert.Equal(t, expectedImageString, actualImageString)
}
1 change: 0 additions & 1 deletion pkg/resourcegenerator/resourceutils/metadata_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ func TestSetResourceLabels(t *testing.T) {

expectedLabels := map[string]string{
"app.kubernetes.io/name": "testapp",
"app.kubernetes.io/version": "latest",
"app.kubernetes.io/managed-by": "skiperator",
"skiperator.kartverket.no/controller": "application",
"application.skiperator.no/app": "testapp",
Expand Down
2 changes: 2 additions & 0 deletions pkg/resourcegenerator/service/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
skiperatorv1alpha1 "github.com/kartverket/skiperator/api/v1alpha1"
"github.com/kartverket/skiperator/api/v1alpha1/podtypes"
"github.com/kartverket/skiperator/pkg/reconciliation"
"github.com/kartverket/skiperator/pkg/resourcegenerator/resourceutils"
"github.com/kartverket/skiperator/pkg/util"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -36,6 +37,7 @@ func Generate(r reconciliation.Reconciliation) error {

service := corev1.Service{ObjectMeta: metav1.ObjectMeta{Namespace: application.Namespace, Name: application.Name}}
service.Labels = util.GetPodAppSelector(application.Name)
service.Labels["app.kubernetes.io/version"] = resourceutils.GetImageVersion(application.Spec.Image)

ports := append(getAdditionalPorts(application.Spec.AdditionalPorts), getServicePort(application.Spec.Port, application.Spec.AppProtocol))
if r.IsIstioEnabled() {
Expand Down
13 changes: 13 additions & 0 deletions tests/application/labels-imageversion/application-assert.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
apiVersion: v1
kind: Service
metadata:
labels:
app.kubernetes.io/version: "1234567890123456"

---

apiVersion: v1
kind: Pod
metadata:
labels:
app.kubernetes.io/version: "1234567890123456"
7 changes: 7 additions & 0 deletions tests/application/labels-imageversion/application-patch.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
apiVersion: skiperator.kartverket.no/v1alpha1
kind: Application
metadata:
name: imageversionshalabel
spec:
image: image@sha256:1234567890123456
port: 8080
7 changes: 7 additions & 0 deletions tests/application/labels-imageversion/application.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
apiVersion: skiperator.kartverket.no/v1alpha1
kind: Application
metadata:
name: imageversionlabellatest
spec:
image: image
port: 8080
16 changes: 16 additions & 0 deletions tests/application/labels-imageversion/chainsaw-test.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
apiVersion: chainsaw.kyverno.io/v1alpha1
kind: Test
metadata:
name: resource-label
spec:
skip: false
concurrent: true
skipDelete: false
steps:
- try:
- create:
file: application.yaml
- apply:
file: application-patch.yaml
- assert:
file: application-assert.yaml
6 changes: 6 additions & 0 deletions tests/application/minimal/application-assert.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ metadata:
annotations:
argocd.argoproj.io/sync-options: "Prune=false"
labels:
app.kubernetes.io/name: minimal
app.kubernetes.io/managed-by: "skiperator"
skiperator.kartverket.no/controller: "application"
application.skiperator.no/app: minimal
Expand All @@ -23,6 +24,7 @@ kind: Deployment
metadata:
name: minimal
labels:
app.kubernetes.io/name: minimal
app.kubernetes.io/managed-by: "skiperator"
skiperator.kartverket.no/controller: "application"
application.skiperator.no/app: minimal
Expand Down Expand Up @@ -123,6 +125,7 @@ metadata:
annotations:
argocd.argoproj.io/sync-options: "Prune=false"
labels:
app.kubernetes.io/name: minimal
app.kubernetes.io/managed-by: "skiperator"
skiperator.kartverket.no/controller: "application"
application.skiperator.no/app: minimal
Expand Down Expand Up @@ -156,6 +159,8 @@ metadata:
annotations:
argocd.argoproj.io/sync-options: "Prune=false"
labels:
app.kubernetes.io/name: minimal
app.kubernetes.io/version: latest
app.kubernetes.io/managed-by: "skiperator"
skiperator.kartverket.no/controller: "application"
application.skiperator.no/app: minimal
Expand Down Expand Up @@ -185,6 +190,7 @@ metadata:
annotations:
argocd.argoproj.io/sync-options: "Prune=false"
labels:
app.kubernetes.io/name: minimal
app.kubernetes.io/managed-by: "skiperator"
skiperator.kartverket.no/controller: "application"
application.skiperator.no/app: minimal
Expand Down
1 change: 1 addition & 0 deletions tests/skipjob/minimal-job/skipjob-assert.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ metadata:
labels:
skiperator.kartverket.no/skipjob: "true"
skiperator.kartverket.no/skipjobName: minimal-job
app.kubernetes.io/version: "5.34.0"
app.kubernetes.io/managed-by: "skiperator"
skiperator.kartverket.no/controller: "skipjob"
spec:
Expand Down

0 comments on commit a4bba73

Please sign in to comment.