DEV-55114 Argus shouldn't overwrite system.categories values

Author: JeremyCD

pkg/device/builder/builder.go

@@ -101,16 +101,13 @@ func setProperty(name, value string) types.DeviceOption {
 }
 
 func getUpdatedSystemCategories(oldValue, newValue string) string {
-	newValues := strings.Split(newValue, ",")
-	var category string
-	for _, nv := range newValues {
-		if !strings.Contains(nv, "=") {
-			category = nv
-			break
+	// we do not use strings.contain, because it may match as substring of some prop
+	oldValues := strings.Split(strings.TrimSpace(oldValue), ",")
+	for _, ov := range oldValues {
+		if ov == newValue {
+			return oldValue
 		}
 	}
-	if !strings.Contains(oldValue, category) {
-		oldValue = oldValue + "," + category
-	}
+	oldValue = oldValue + "," + newValue
 	return oldValue
 }

pkg/device/builder/builder_test.go

@@ -27,7 +27,7 @@ func TestBuilder_SetProperty(t *testing.T) {
 	}
 
 	sysPropValue1 := "k1=v1,k2=v2"
-	sysPropValue2 := constants.PodCategory + ",k2=v21,k3=v3"
+	sysPropValue2 := constants.PodCategory
 
 	setProp = setProperty(constants.K8sSystemCategoriesPropertyKey, sysPropValue1)
 	setProp(device)
@@ -42,6 +42,11 @@ func TestBuilder_SetProperty(t *testing.T) {
 	if sysPropValue1+","+constants.PodCategory != getDevicePropValueByName(device, constants.K8sSystemCategoriesPropertyKey) {
 		t.Errorf("failed to set prop %s:%s to the device", constants.K8sSystemCategoriesPropertyKey, sysPropValue2)
 	}
+	setProp = setProperty(constants.K8sSystemCategoriesPropertyKey, sysPropValue2)
+	setProp(device)
+	if sysPropValue1+","+constants.PodCategory != getDevicePropValueByName(device, constants.K8sSystemCategoriesPropertyKey) {
+		t.Errorf("failed to set prop %s:%s to the device", constants.K8sSystemCategoriesPropertyKey, sysPropValue2)
+	}
 }
 
 func getDevicePropValueByName(d *models.Device, name string) string {

pkg/etcd/etcd.go

@@ -10,7 +10,6 @@ import (
 	"github.com/coreos/etcd/client"
 	"github.com/logicmonitor/k8s-argus/pkg/constants"
 	"github.com/logicmonitor/k8s-argus/pkg/types"
-	"github.com/logicmonitor/k8s-argus/pkg/utilities"
 	log "github.com/sirupsen/logrus"
 )
 
@@ -86,11 +85,10 @@ func (c *Controller) addDevice(member *Member) {
 
 // nolint: unparam
 func (c *Controller) args(member *Member, category string) []types.DeviceOption {
-	categories := utilities.BuildSystemCategoriesFromLabels(category, nil)
 	return []types.DeviceOption{
 		c.Name(member.URL.Hostname()),
 		c.DisplayName(fmtMemberDisplayName(member)),
-		c.SystemCategories(categories),
+		c.SystemCategories(category),
 		c.Auto("clientport", member.URL.Port()),
 	}
 }

pkg/utilities/utilities.go

@@ -4,14 +4,6 @@ import (
 	"regexp"
 )
 
-// BuildSystemCategoriesFromLabels formats a system.categories string.
-func BuildSystemCategoriesFromLabels(categories string, labels map[string]string) string {
-	for k, v := range labels {
-		categories += "," + k + "=" + v
-	}
-	return categories
-}
-
 // GetLabelByPrefix takes a list of labels returns the first label matching the specified prefix
 func GetLabelByPrefix(prefix string, labels map[string]string) (string, string) {
 	for k, v := range labels {

pkg/watch/deployment/deployment.go

@@ -9,7 +9,6 @@ import (
 	"github.com/logicmonitor/k8s-argus/pkg/constants"
 	"github.com/logicmonitor/k8s-argus/pkg/permission"
 	"github.com/logicmonitor/k8s-argus/pkg/types"
-	"github.com/logicmonitor/k8s-argus/pkg/utilities"
 	log "github.com/sirupsen/logrus"
 	"k8s.io/api/apps/v1beta2"
 	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -116,12 +115,11 @@ func (w *Watcher) move(deployment *v1beta2.Deployment) {
 }
 
 func (w *Watcher) args(deployment *v1beta2.Deployment, category string) []types.DeviceOption {
-	categories := utilities.BuildSystemCategoriesFromLabels(category, deployment.Labels)
 	return []types.DeviceOption{
 		w.Name(deployment.Name),
 		w.ResourceLabels(deployment.Labels),
 		w.DisplayName(fmtDeploymentDisplayName(deployment)),
-		w.SystemCategories(categories),
+		w.SystemCategories(category),
 		w.Auto("name", deployment.Name),
 		w.Auto("namespace", deployment.Namespace),
 		w.Auto("selflink", deployment.SelfLink),

pkg/watch/node/node.go

@@ -147,12 +147,11 @@ func (w *Watcher) move(node *v1.Node) {
 }
 
 func (w *Watcher) args(node *v1.Node, category string) []types.DeviceOption {
-	categories := utilities.BuildSystemCategoriesFromLabels(category, node.Labels)
 	return []types.DeviceOption{
 		w.Name(getInternalAddress(node.Status.Addresses).Address),
 		w.ResourceLabels(node.Labels),
 		w.DisplayName(node.Name),
-		w.SystemCategories(categories),
+		w.SystemCategories(category),
 		w.Auto("name", node.Name),
 		w.Auto("selflink", node.SelfLink),
 		w.Auto("uid", string(node.UID)),

pkg/watch/pod/pod.go

@@ -7,7 +7,6 @@ import (
 
 	"github.com/logicmonitor/k8s-argus/pkg/constants"
 	"github.com/logicmonitor/k8s-argus/pkg/types"
-	"github.com/logicmonitor/k8s-argus/pkg/utilities"
 	log "github.com/sirupsen/logrus"
 	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -144,12 +143,11 @@ func (w *Watcher) move(pod *v1.Pod) {
 }
 
 func (w *Watcher) args(pod *v1.Pod, category string) []types.DeviceOption {
-	categories := utilities.BuildSystemCategoriesFromLabels(category, pod.Labels)
 	options := []types.DeviceOption{
 		w.Name(getPodDNSName(pod)),
 		w.ResourceLabels(pod.Labels),
 		w.DisplayName(pod.Name),
-		w.SystemCategories(categories),
+		w.SystemCategories(category),
 		w.Auto("name", pod.Name),
 		w.Auto("namespace", pod.Namespace),
 		w.Auto("nodename", pod.Spec.NodeName),

pkg/watch/service/service.go

@@ -8,7 +8,6 @@ import (
 
 	"github.com/logicmonitor/k8s-argus/pkg/constants"
 	"github.com/logicmonitor/k8s-argus/pkg/types"
-	"github.com/logicmonitor/k8s-argus/pkg/utilities"
 	log "github.com/sirupsen/logrus"
 	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -132,12 +131,11 @@ func (w *Watcher) move(service *v1.Service) {
 }
 
 func (w *Watcher) args(service *v1.Service, category string) []types.DeviceOption {
-	categories := utilities.BuildSystemCategoriesFromLabels(category, service.Labels)
 	return []types.DeviceOption{
 		w.Name(service.Spec.ClusterIP),
 		w.ResourceLabels(service.Labels),
 		w.DisplayName(fmtServiceDisplayName(service)),
-		w.SystemCategories(categories),
+		w.SystemCategories(category),
 		w.Auto("name", service.Name),
 		w.Auto("namespace", service.Namespace),
 		w.Auto("selflink", service.SelfLink),