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

chore: use generic Set instead of deprecated sets.String #422

Merged
merged 2 commits into from
Sep 13, 2023
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
5 changes: 0 additions & 5 deletions .golangci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -243,11 +243,6 @@ issues:
- staticcheck
text: "SA9003:"

# Exclude some deprecation errors
- linters:
- staticcheck
text: "SA1019:"

# Exclude lll issues for long lines with go:generate
- linters:
- lll
Expand Down
4 changes: 3 additions & 1 deletion controllers/modelmesh/constraints.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ import (
"errors"
"strings"

"k8s.io/apimachinery/pkg/util/sets"

kserveapi "github.com/kserve/kserve/pkg/apis/serving/v1alpha1"
appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -68,5 +70,5 @@ func (m *Deployment) addModelTypeConstraints(deployment *appsv1.Deployment) erro
}

func generateLabelsEnvVar(rts *kserveapi.ServingRuntimeSpec, restProxyEnabled bool, rtName string) string {
return strings.Join(GetServingRuntimeLabelSet(rts, restProxyEnabled, rtName).List(), ",")
return strings.Join(sets.List(GetServingRuntimeLabelSet(rts, restProxyEnabled, rtName)), ",")
}
8 changes: 4 additions & 4 deletions controllers/modelmesh/model_type_labels.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,10 @@ import (
)

func GetServingRuntimeLabelSets(rt *kserveapi.ServingRuntimeSpec, restProxyEnabled bool, rtName string) (
mtLabels sets.String, pvLabels sets.String, rtLabel string) {
mtLabels sets.Set[string], pvLabels sets.Set[string], rtLabel string) {

// model type labels
mtSet := make(sets.String, 2*len(rt.SupportedModelFormats))
mtSet := make(sets.Set[string], 2*len(rt.SupportedModelFormats))
for _, t := range rt.SupportedModelFormats {
// only include model type labels when autoSelect is true
if t.AutoSelect != nil && *t.AutoSelect {
Expand All @@ -38,7 +38,7 @@ func GetServingRuntimeLabelSets(rt *kserveapi.ServingRuntimeSpec, restProxyEnabl
}
}
// protocol versions
pvSet := make(sets.String, len(rt.ProtocolVersions))
pvSet := make(sets.Set[string], len(rt.ProtocolVersions))
for _, pv := range rt.ProtocolVersions {
pvSet.Insert(fmt.Sprintf("pv:%s", pv))
if restProxyEnabled && pv == constants.ProtocolGRPCV2 {
Expand All @@ -49,7 +49,7 @@ func GetServingRuntimeLabelSets(rt *kserveapi.ServingRuntimeSpec, restProxyEnabl
return mtSet, pvSet, fmt.Sprintf("rt:%s", rtName)
}

func GetServingRuntimeLabelSet(rt *kserveapi.ServingRuntimeSpec, restProxyEnabled bool, rtName string) sets.String {
func GetServingRuntimeLabelSet(rt *kserveapi.ServingRuntimeSpec, restProxyEnabled bool, rtName string) sets.Set[string] {
s1, s2, l := GetServingRuntimeLabelSets(rt, restProxyEnabled, rtName)
s1 = s1.Union(s2)
s1.Insert(l)
Expand Down
10 changes: 6 additions & 4 deletions controllers/modelmesh/model_type_labels_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ import (
"sort"
"testing"

"k8s.io/apimachinery/pkg/util/sets"

kserveapi "github.com/kserve/kserve/pkg/apis/serving/v1alpha1"
"github.com/kserve/kserve/pkg/constants"
api "github.com/kserve/modelmesh-serving/apis/serving/v1alpha1"
Expand Down Expand Up @@ -82,11 +84,11 @@ func TestGetServingRuntimeLabelSets(t *testing.T) {
if expectedRtLabel != rtLabel {
t.Errorf("Missing expected entry [%s] in set: %v", expectedRtLabel, rtLabel)
}
if !reflect.DeepEqual(mtLabelSet.List(), expectedMtLabels) {
t.Errorf("Labels [%s] don't match expected: %v", mtLabelSet.List(), expectedMtLabels)
if !reflect.DeepEqual(sets.List(mtLabelSet), expectedMtLabels) {
t.Errorf("Labels [%s] don't match expected: %v", sets.List(mtLabelSet), expectedMtLabels)
}
if !reflect.DeepEqual(pvLabelSet.List(), expectedPvLabels) {
t.Errorf("Labels [%s] don't match expected: %v", pvLabelSet.List(), expectedPvLabels)
if !reflect.DeepEqual(sets.List(pvLabelSet), expectedPvLabels) {
t.Errorf("Labels [%s] don't match expected: %v", sets.List(pvLabelSet), expectedPvLabels)
}
}

Expand Down
12 changes: 6 additions & 6 deletions controllers/servingruntime_validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ func validateVolumes(rts *kserveapi.ServingRuntimeSpec, _ *config.Config) error
return nil
}

func checkName(name string, internalNames sets.String, logStr string) error {
func checkName(name string, internalNames sets.Set[string], logStr string) error {
if internalNames.Has(name) {
return fmt.Errorf("%s %s is reserved for internal use", logStr, name)
}
Expand All @@ -151,29 +151,29 @@ func checkName(name string, internalNames sets.String, logStr string) error {
return nil
}

var internalContainerNames = sets.NewString(
var internalContainerNames = sets.New[string](
modelmesh.ModelMeshContainerName,
modelmesh.RESTProxyContainerName,
modelmesh.PullerContainerName,
)

var internalOnlyVolumeMounts = sets.NewString(
var internalOnlyVolumeMounts = sets.New[string](
modelmesh.ConfigStorageMount,
modelmesh.EtcdVolume,
modelmesh.InternalConfigMapName,
modelmesh.SocketVolume,
)

var internalNamedPorts = sets.NewString("grpc", "http", "prometheus")
var internalNamedPorts = sets.New[string]("grpc", "http", "prometheus")

var internalPorts = sets.NewInt32(
var internalPorts = sets.New[int32](
8080, // is used for LiteLinks communication in Model Mesh
8085, // is the port the built-in adapter listens on
8089, // is used for Model Mesh probes
8090, // is used for default preStop hooks
)

var internalVolumes = sets.NewString(
var internalVolumes = sets.New[string](
modelmesh.ConfigStorageMount,
modelmesh.EtcdVolume,
modelmesh.InternalConfigMapName,
Expand Down