Skip to content

Commit

Permalink
chore: add managed by label to namespace (#604)
Browse files Browse the repository at this point in the history
# Description

When creating a namespace object in Kubernetes, add label
“{prefix}/managed: true”. This will help when we want to modify
resources created by CaraML services in a shared cluster, e.g.
configuring log for the managed namespace only

By default the namespace prefix would `caraml.dev/`, and it can be
configured in the config yaml


# Modifications
- As the creation of label for Kubernetes’ object in Merlin previously
is bind to metadata struct, while namespace doesn’t have metada, in this
changes the functionality of labelling (labeller) is moved to it’s own
package
- Add one more prefix in `InitKubernetesLabeller` initiation
- Add the `{prefix}/managed: true` when creating namespace

# Tests

# Checklist
- [x] Added PR label
- [ ] Added unit test, integration, and/or e2e tests
- [x] Tested locally
- [ ] Updated documentation
- [ ] Update Swagger spec if the PR introduce API changes
- [ ] Regenerated Golang and Python client if the PR introduces API
changes

# Release Notes

```release-note

```
  • Loading branch information
bthari authored Sep 12, 2024
1 parent fa928c3 commit a9184ed
Show file tree
Hide file tree
Showing 18 changed files with 203 additions and 133 deletions.
5 changes: 3 additions & 2 deletions api/batch/resource_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"testing"

"github.com/GoogleCloudPlatform/spark-on-k8s-operator/pkg/apis/sparkoperator.k8s.io/v1beta2"
"github.com/caraml-dev/merlin/cluster/labeller"
"github.com/stretchr/testify/assert"
v12 "k8s.io/api/core/v1"
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -134,11 +135,11 @@ var (
)

func TestCreateSparkApplicationResource(t *testing.T) {
err := models.InitKubernetesLabeller("gojek.com/", testEnvironmentName)
err := labeller.InitKubernetesLabeller("gojek.com/", "caraml.dev/", testEnvironmentName)
assert.NoError(t, err)

defer func() {
_ = models.InitKubernetesLabeller("", "")
_ = labeller.InitKubernetesLabeller("", "", "")
}()

tests := []struct {
Expand Down
3 changes: 2 additions & 1 deletion api/cluster/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"testing"
"time"

"github.com/caraml-dev/merlin/cluster/labeller"
kservev1beta1 "github.com/kserve/kserve/pkg/apis/serving/v1beta1"
fakekserve "github.com/kserve/kserve/pkg/client/clientset/versioned/fake"
fakekservev1beta1 "github.com/kserve/kserve/pkg/client/clientset/versioned/typed/serving/v1beta1/fake"
Expand Down Expand Up @@ -759,7 +760,7 @@ func TestController_DeployInferenceService(t *testing.T) {
}

func TestController_DeployInferenceService_PodDisruptionBudgetsRemoval(t *testing.T) {
err := models.InitKubernetesLabeller("gojek.com/", "dev")
err := labeller.InitKubernetesLabeller("gojek.com/", "caraml.dev/", "dev")
assert.Nil(t, err)

minAvailablePercentage := 80
Expand Down
84 changes: 84 additions & 0 deletions api/cluster/labeller/labeller.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
package labeller

import (
"fmt"
"regexp"
)

const (
LabelAppName = "app"
LabelComponent = "component"
LabelEnvironment = "environment"
LabelOrchestratorName = "orchestrator"
LabelStreamName = "stream"
LabelTeamName = "team"
LabelManaged = "managed"
)

var reservedKeys = map[string]bool{
LabelAppName: true,
LabelComponent: true,
LabelEnvironment: true,
LabelOrchestratorName: true,
LabelStreamName: true,
LabelTeamName: true,
}

var (
prefix string
nsPrefix string
environment string
validPrefixRegex = regexp.MustCompile("^(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?(/)$")
)

// InitKubernetesLabeller builds a new KubernetesLabeller Singleton
func InitKubernetesLabeller(p, ns, e string) error {
if err := isValidPrefix(p, "prefix"); err != nil {
return err
}
if err := isValidPrefix(ns, "namespace prefix"); err != nil {
return err
}

prefix = p
nsPrefix = ns
environment = e
return nil
}

// isValidPrefix checks if the given prefix is valid
func isValidPrefix(prefix, name string) error {
if len(prefix) > 253 {
return fmt.Errorf("length of %s is greater than 253 characters", name)
}
if isValidPrefix := validPrefixRegex.MatchString(prefix); !isValidPrefix {
return fmt.Errorf("%s name violates kubernetes label's prefix constraint", name)
}
return nil
}

// GetLabelName prefixes the label with the config specified label and returns the formatted label prefix
func GetLabelName(name string) string {
return fmt.Sprintf("%s%s", prefix, name)
}

// GetNamespaceLabel prefixes the label with the config specified namespace label and returns the formatted label prefix
func GetNamespaceLabel(name string) string {
return fmt.Sprintf("%s%s", nsPrefix, name)
}

// GetPrefix returns the labeller prefix
func GetPrefix() string {
return prefix
}

// GetEnvironment returns the environment
func GetEnvironment() string {
return environment
}

// IsReservedKey checks if the key is a reserved key
func IsReservedKey(key string) bool {
_, usingReservedKeys := reservedKeys[key]
return usingReservedKeys
}
62 changes: 62 additions & 0 deletions api/cluster/labeller/labeller_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
package labeller

import (
"testing"

"github.com/stretchr/testify/assert"
)

func TestInitKubernetesLabeller(t *testing.T) {
defaultNsPrefix := "caraml.dev/"
err := InitKubernetesLabeller("gojek.com/", "caraml.dev/", "dev")
assert.NoError(t, err)

defer func() {
_ = InitKubernetesLabeller("", "", "")
}()

tests := []struct {
prefix string
wantErr bool
}{
{
"gojek.com/",
false,
},
{
"model.caraml.dev/",
false,
},
{
"goto/gojek",
true,
},
{
"gojek",
true,
},
{
"gojek.com/caraml",
true,
},
{
"gojek//",
true,
},
{
"gojek.com//",
true,
},
{
"//gojek.com",
true,
},
}
for _, tt := range tests {
t.Run(tt.prefix, func(t *testing.T) {
if err := InitKubernetesLabeller(tt.prefix, defaultNsPrefix, "dev"); (err != nil) != tt.wantErr {
t.Errorf("InitKubernetesLabeller() error = %v, wantErr %v", err, tt.wantErr)
}
})
}
}
5 changes: 5 additions & 0 deletions api/cluster/namespace.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,10 @@ package cluster
import (
"context"
"fmt"
"strconv"
"time"

"github.com/caraml-dev/merlin/cluster/labeller"
corev1 "k8s.io/api/core/v1"
kerrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -59,6 +61,9 @@ func (k *namespaceCreator) CreateNamespace(ctx context.Context, namespace string
return k.Namespaces().Create(ctx, &corev1.Namespace{
ObjectMeta: metav1.ObjectMeta{
Name: namespace,
Labels: map[string]string{
labeller.GetNamespaceLabel(labeller.LabelManaged): strconv.FormatBool(true),
},
},
}, metav1.CreateOptions{})
}
5 changes: 3 additions & 2 deletions api/cluster/pdb_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"reflect"
"testing"

"github.com/caraml-dev/merlin/cluster/labeller"
"github.com/stretchr/testify/assert"
policyv1 "k8s.io/api/policy/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand All @@ -14,7 +15,7 @@ import (
)

func Test_NewPodDisruptionBudget(t *testing.T) {
err := models.InitKubernetesLabeller("gojek.com/", "dev")
err := labeller.InitKubernetesLabeller("gojek.com/", "caraml.dev/", "dev")
assert.Nil(t, err)

twenty := 20
Expand Down Expand Up @@ -190,7 +191,7 @@ func TestPodDisruptionBudget_BuildPDBSpec(t *testing.T) {
}

func Test_generatePDBSpecs(t *testing.T) {
err := models.InitKubernetesLabeller("gojek.com/", "dev")
err := labeller.InitKubernetesLabeller("gojek.com/", "caraml.dev/", "dev")
assert.Nil(t, err)

ten, twenty := 10, 20
Expand Down
5 changes: 3 additions & 2 deletions api/cluster/resource/templater_gpu_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"fmt"
"testing"

"github.com/caraml-dev/merlin/cluster/labeller"
kservev1beta1 "github.com/kserve/kserve/pkg/apis/serving/v1beta1"
kserveconstant "github.com/kserve/kserve/pkg/constants"
"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -66,11 +67,11 @@ var (
)

func TestCreateInferenceServiceSpecWithGPU(t *testing.T) {
err := models.InitKubernetesLabeller("gojek.com/", testEnvironmentName)
err := labeller.InitKubernetesLabeller("gojek.com/", "caraml.dev/", testEnvironmentName)
assert.NoError(t, err)

defer func() {
_ = models.InitKubernetesLabeller("", "")
_ = labeller.InitKubernetesLabeller("", "", "")
}()

project := mlp.Project{
Expand Down
17 changes: 9 additions & 8 deletions api/cluster/resource/templater_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"testing"
"time"

"github.com/caraml-dev/merlin/cluster/labeller"
kservev1beta1 "github.com/kserve/kserve/pkg/apis/serving/v1beta1"
kserveconstant "github.com/kserve/kserve/pkg/constants"
"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -206,11 +207,11 @@ var (
)

func TestCreateInferenceServiceSpec(t *testing.T) {
err := models.InitKubernetesLabeller("gojek.com/", testEnvironmentName)
err := labeller.InitKubernetesLabeller("gojek.com/", "caraml.dev/", testEnvironmentName)
assert.NoError(t, err)

defer func() {
_ = models.InitKubernetesLabeller("", "")
_ = labeller.InitKubernetesLabeller("", "", "")
}()

project := mlp.Project{
Expand Down Expand Up @@ -1935,11 +1936,11 @@ func TestCreateInferenceServiceSpec(t *testing.T) {
}

func TestCreateInferenceServiceSpecWithTransformer(t *testing.T) {
err := models.InitKubernetesLabeller("gojek.com/", testEnvironmentName)
err := labeller.InitKubernetesLabeller("gojek.com/", "caraml.dev/", testEnvironmentName)
assert.NoError(t, err)

defer func() {
_ = models.InitKubernetesLabeller("", "")
_ = labeller.InitKubernetesLabeller("", "", "")
}()

project := mlp.Project{
Expand Down Expand Up @@ -2897,11 +2898,11 @@ func TestCreateInferenceServiceSpecWithTransformer(t *testing.T) {
}

func TestCreateInferenceServiceSpecWithLogger(t *testing.T) {
err := models.InitKubernetesLabeller("gojek.com/", testEnvironmentName)
err := labeller.InitKubernetesLabeller("gojek.com/", "caraml.dev/", testEnvironmentName)
assert.NoError(t, err)

defer func() {
_ = models.InitKubernetesLabeller("", "")
_ = labeller.InitKubernetesLabeller("", "", "")
}()

project := mlp.Project{
Expand Down Expand Up @@ -3390,11 +3391,11 @@ func TestCreateInferenceServiceSpecWithLogger(t *testing.T) {
}

func TestCreateInferenceServiceSpecWithTopologySpreadConstraints(t *testing.T) {
err := models.InitKubernetesLabeller("gojek.com/", testEnvironmentName)
err := labeller.InitKubernetesLabeller("gojek.com/", "caraml.dev/", testEnvironmentName)
assert.NoError(t, err)

defer func() {
_ = models.InitKubernetesLabeller("", "")
_ = labeller.InitKubernetesLabeller("", "", "")
}()

project := mlp.Project{
Expand Down
4 changes: 2 additions & 2 deletions api/cmd/api/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"syscall"
"time"

"github.com/caraml-dev/merlin/cluster/labeller"
mlflowDelete "github.com/caraml-dev/mlp/api/pkg/client/mlflow"
"github.com/caraml-dev/mlp/api/pkg/instrumentation/sentry"
webhookManager "github.com/caraml-dev/mlp/api/pkg/webhooks"
Expand All @@ -41,7 +42,6 @@ import (
"github.com/caraml-dev/merlin/database"
"github.com/caraml-dev/merlin/log"
mlflow "github.com/caraml-dev/merlin/mlflow"
"github.com/caraml-dev/merlin/models"
"github.com/caraml-dev/merlin/pkg/gitlab"
"github.com/caraml-dev/merlin/pkg/observability/event"
"github.com/caraml-dev/merlin/queue"
Expand Down Expand Up @@ -264,7 +264,7 @@ func buildDependencies(ctx context.Context, cfg *config.Config, db *gorm.DB, dis
mlpAPIClient := initMLPAPIClient(ctx, cfg.MlpAPIConfig)
coreClient := initFeastCoreClient(cfg.StandardTransformerConfig.FeastCoreURL, cfg.StandardTransformerConfig.FeastCoreAuthAudience, cfg.StandardTransformerConfig.EnableAuth)

if err := models.InitKubernetesLabeller(cfg.DeploymentLabelPrefix, cfg.Environment); err != nil {
if err := labeller.InitKubernetesLabeller(cfg.DeploymentLabelPrefix, cfg.NamespaceLabelPrefix, cfg.Environment); err != nil {
log.Panicf("invalid deployment label prefix (%s): %s", cfg.DeploymentLabelPrefix, err)
}

Expand Down
1 change: 1 addition & 0 deletions api/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ type Config struct {
NumOfQueueWorkers int `validate:"required" default:"2"`
SwaggerPath string `validate:"required" default:"./swagger.yaml"`

NamespaceLabelPrefix string `validate:"required" default:"caraml.dev/"`
DeploymentLabelPrefix string `validate:"required" default:"gojek.com/"`
PyfuncGRPCOptions string `validate:"required" default:"{}"`

Expand Down
1 change: 1 addition & 0 deletions api/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -366,6 +366,7 @@ func TestLoad(t *testing.T) {
},
NumOfQueueWorkers: 2,
SwaggerPath: "swaggerpath.com",
NamespaceLabelPrefix: "caraml.dev/",
DeploymentLabelPrefix: "caraml.com/",
PyfuncGRPCOptions: "{}",
DbConfig: DatabaseConfig{
Expand Down
Loading

0 comments on commit a9184ed

Please sign in to comment.