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

SKIP-1159 - Add recommended labels. #527

Merged
merged 14 commits into from
Oct 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions api/v1alpha1/application_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -417,6 +417,7 @@ func (a *Application) SetStatus(status SkiperatorStatus) {
// TODO clean up labels
func (a *Application) GetDefaultLabels() map[string]string {
return map[string]string{
"app.kubernetes.io/name": a.Name,
"app.kubernetes.io/managed-by": "skiperator",
"skiperator.kartverket.no/controller": "application",
"application.skiperator.no/app": a.Name,
Expand Down
1 change: 1 addition & 0 deletions api/v1alpha1/routing_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ func (in *Routing) SetStatus(status SkiperatorStatus) {

func (in *Routing) GetDefaultLabels() map[string]string {
return map[string]string{
"app.kubernetes.io/name": in.Name,
"app.kubernetes.io/managed-by": "skiperator",
"skiperator.kartverket.no/controller": "routing",
"skiperator.kartverket.no/routing-name": in.Name,
Expand Down
1 change: 1 addition & 0 deletions api/v1alpha1/skipjob_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,7 @@ func (skipJob *SKIPJob) FillDefaultStatus() {

func (skipJob *SKIPJob) GetDefaultLabels() map[string]string {
return map[string]string{
"app.kubernetes.io/name": skipJob.Name,
"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
1 change: 1 addition & 0 deletions api/v1alpha1/skipns_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ func (n SKIPNamespace) SetStatus(status SkiperatorStatus) {}

func (n SKIPNamespace) GetDefaultLabels() map[string]string {
return map[string]string{
"app.kubernetes.io/name": n.Name,
"app.kubernetes.io/managed-by": "skiperator",
"skiperator.kartverket.no/controller": "namespace",
}
Expand Down
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
22 changes: 19 additions & 3 deletions 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,8 +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: make(map[string]string),
}

setJobLabels(skipJob, meta.Labels)
job := batchv1.Job{ObjectMeta: meta}
cronJob := batchv1.CronJob{ObjectMeta: meta}

Expand Down Expand Up @@ -71,7 +74,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()
setJobLabels(skipJob, spec.JobTemplate.Labels)

return spec
}
Expand Down Expand Up @@ -126,7 +129,20 @@ 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()

setJobLabels(skipJob, jobSpec.Template.Labels)

return jobSpec
}

//func getSkipJobVersion(skipJob *skiperatorv1alpha1.SKIPJob) string {
// if skipJob.Spec.Container.Image != "" {
// return resourceutils.GetImageVersion(skipJob.Spec.Container.Image)
// }
// return ""
//}

func setJobLabels(skipJob *skiperatorv1alpha1.SKIPJob, labels map[string]string) {
labels["app"] = skipJob.KindPostFixedName()
labels["app.kubernetes.io/version"] = resourceutils.GetImageVersion(skipJob.Spec.Container.Image)
}
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)
}
13 changes: 11 additions & 2 deletions pkg/resourcegenerator/resourceutils/metadata_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,17 @@ func TestSetResourceLabels(t *testing.T) {
},
}

expectedLabels := map[string]string{
"app.kubernetes.io/name": "testapp",
"app.kubernetes.io/managed-by": "skiperator",
"skiperator.kartverket.no/controller": "application",
"application.skiperator.no/app": "testapp",
"application.skiperator.no/app-name": "testapp",
"application.skiperator.no/app-namespace": "testns",
"someLabel": "someValue",
}

SetApplicationLabels(sa, app)
assert.True(t, len(sa.GetLabels()) == 6)
assert.True(t, sa.GetLabels()["someLabel"] == "someValue")
assert.Equal(t, expectedLabels, sa.GetLabels())
assert.Empty(t, sa.GetLabels()["otherLabel"])
}
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"
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
2 changes: 2 additions & 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 Expand Up @@ -56,6 +57,7 @@ kind: Pod
metadata:
labels:
app: minimal-job-skipjob
app.kubernetes.io/version: "5.34.0"
app.kubernetes.io/managed-by: skiperator
batch.kubernetes.io/job-name: minimal-job
job-name: minimal-job
Expand Down
Loading