diff --git a/pkg/constants/constants.go b/pkg/constants/constants.go index dc01bd3fb..c34f5b89f 100644 --- a/pkg/constants/constants.go +++ b/pkg/constants/constants.go @@ -90,6 +90,8 @@ const ( K8sResourceCreatedOnPropertyKey = "kubernetes.resourceCreatedOn" // K8sDeviceType is the type value of the k8s device K8sDeviceType = 8 + // K8sSystemCategoriesPropertyKey is the key of the unique custom property kubernetes system categories + K8sSystemCategoriesPropertyKey = "system.categories" ) const ( diff --git a/pkg/device/builder/builder.go b/pkg/device/builder/builder.go index 3dd5b64e7..57fdc20b9 100644 --- a/pkg/device/builder/builder.go +++ b/pkg/device/builder/builder.go @@ -1,6 +1,8 @@ package builder import ( + "strings" + "github.com/logicmonitor/k8s-argus/pkg/constants" "github.com/logicmonitor/k8s-argus/pkg/types" "github.com/logicmonitor/lm-sdk-go/models" @@ -35,22 +37,38 @@ func (b *Builder) CollectorID(id int32) types.DeviceOption { // SystemCategories implements types.DeviceBuilder func (b *Builder) SystemCategories(categories string) types.DeviceOption { - return setProperty("system.categories", categories) + return setProperty(constants.K8sSystemCategoriesPropertyKey, categories) } // ResourceLabels implements types.DeviceBuilder func (b *Builder) ResourceLabels(properties map[string]string) types.DeviceOption { return func(device *models.Device) { + if device == nil { + return + } + if device.CustomProperties == nil { + device.CustomProperties = []*models.NameAndValue{} + } for name, value := range properties { propName := constants.LabelCustomPropertyPrefix + name propValue := value if propValue == "" { propValue = constants.LabelNullPlaceholder } - device.CustomProperties = append(device.CustomProperties, &models.NameAndValue{ - Name: &propName, - Value: &propValue, - }) + existed := false + for _, prop := range device.CustomProperties { + if *prop.Name == propName { + *prop.Value = propValue + existed = true + break + } + } + if !existed { + device.CustomProperties = append(device.CustomProperties, &models.NameAndValue{ + Name: &propName, + Value: &propValue, + }) + } } } } @@ -72,6 +90,21 @@ func (b *Builder) Custom(name, value string) types.DeviceOption { func setProperty(name, value string) types.DeviceOption { return func(device *models.Device) { + if device == nil { + return + } + if device.CustomProperties == nil { + device.CustomProperties = []*models.NameAndValue{} + } + for _, prop := range device.CustomProperties { + if *prop.Name == name && value != "" { + if *prop.Name == constants.K8sSystemCategoriesPropertyKey { + value = getUpdatedSystemCategories(*prop.Value, value) + } + *prop.Value = value + return + } + } if value != "" { device.CustomProperties = append(device.CustomProperties, &models.NameAndValue{ Name: &name, @@ -82,3 +115,15 @@ func setProperty(name, value string) types.DeviceOption { } } } + +func getUpdatedSystemCategories(oldValue, newValue string) string { + // we do not use strings.contain, because it may be matched as substring of some prop + oldValues := strings.Split(strings.TrimSpace(oldValue), ",") + for _, ov := range oldValues { + if ov == newValue { + return oldValue + } + } + oldValue = oldValue + "," + newValue + return oldValue +} diff --git a/pkg/device/builder/builder_test.go b/pkg/device/builder/builder_test.go new file mode 100644 index 000000000..b7bb5c8a3 --- /dev/null +++ b/pkg/device/builder/builder_test.go @@ -0,0 +1,62 @@ +package builder + +import ( + "testing" + + "github.com/logicmonitor/k8s-argus/pkg/constants" + "github.com/logicmonitor/lm-sdk-go/models" +) + +func TestBuilder_SetProperty(t *testing.T) { + propName := "name" + propValue1 := "value1" + propValue2 := "value2" + device := &models.Device{ + CustomProperties: []*models.NameAndValue{}, + } + + setProp := setProperty(propName, propValue1) + setProp(device) + if propValue1 != getDevicePropValueByName(device, propName) { + t.Errorf("failed to set prop %s:%s to the device", propName, propValue1) + } + setProp = setProperty(propName, propValue2) + setProp(device) + if propValue2 != getDevicePropValueByName(device, propName) { + t.Errorf("failed to set prop %s:%s to the device", propName, propValue2) + } + + sysPropValue1 := "k1=v1,k2=v2" + sysPropValue2 := constants.PodCategory + + setProp = setProperty(constants.K8sSystemCategoriesPropertyKey, sysPropValue1) + setProp(device) + if propValue2 != getDevicePropValueByName(device, propName) { + t.Errorf("failed to set prop %s:%s to the device", propName, propValue2) + } + if sysPropValue1 != getDevicePropValueByName(device, constants.K8sSystemCategoriesPropertyKey) { + t.Errorf("failed to set prop %s:%s to the device", constants.K8sSystemCategoriesPropertyKey, sysPropValue1) + } + 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) + } + 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 { + if d == nil || d.CustomProperties == nil { + return "" + } + for _, prop := range d.CustomProperties { + if *prop.Name == name { + return *prop.Value + } + } + return "" +} diff --git a/pkg/device/device.go b/pkg/device/device.go index 98af65198..640e4f49a 100644 --- a/pkg/device/device.go +++ b/pkg/device/device.go @@ -22,36 +22,42 @@ type Manager struct { ControllerClient api.CollectorSetControllerClient } -func buildDevice(c *config.Config, client api.CollectorSetControllerClient, options ...types.DeviceOption) *models.Device { - hostGroupIds := "1" - propertyName := constants.K8sClusterNamePropertyKey - // use the copy value - clusterName := c.ClusterName - device := &models.Device{ - CustomProperties: []*models.NameAndValue{ - { - Name: &propertyName, - Value: &clusterName, +func buildDevice(c *config.Config, client api.CollectorSetControllerClient, d *models.Device, options ...types.DeviceOption) *models.Device { + if d == nil { + hostGroupIds := "1" + propertyName := constants.K8sClusterNamePropertyKey + // use the copy value + clusterName := c.ClusterName + d = &models.Device{ + CustomProperties: []*models.NameAndValue{ + { + Name: &propertyName, + Value: &clusterName, + }, }, - }, - DisableAlerting: c.DisableAlerting, - HostGroupIds: &hostGroupIds, - DeviceType: constants.K8sDeviceType, - } + DisableAlerting: c.DisableAlerting, + HostGroupIds: &hostGroupIds, + DeviceType: constants.K8sDeviceType, + } - for _, option := range options { - option(device) - } + for _, option := range options { + option(d) + } - reply, err := client.CollectorID(context.Background(), &api.CollectorIDRequest{}) - if err != nil { - log.Errorf("Failed to get collector ID: %v", err) + reply, err := client.CollectorID(context.Background(), &api.CollectorIDRequest{}) + if err != nil { + log.Errorf("Failed to get collector ID: %v", err) + } else { + log.Infof("Using collector ID %d for %q", reply.Id, *d.DisplayName) + d.PreferredCollectorID = &reply.Id + } } else { - log.Infof("Using collector ID %d for %q", reply.Id, *device.DisplayName) - device.PreferredCollectorID = &reply.Id + for _, option := range options { + option(d) + } } - return device + return d } // checkAndUpdateExistingDevice tries to find and update the devices which needs to be changed @@ -210,7 +216,7 @@ func (m *Manager) FindByDisplayNameAndClusterName(displayName string) (*models.D // Add implements types.DeviceManager. func (m *Manager) Add(options ...types.DeviceOption) (*models.Device, error) { - device := buildDevice(m.Config(), m.ControllerClient, options...) + device := buildDevice(m.Config(), m.ControllerClient, nil, options...) log.Debugf("%#v", device) params := lm.NewAddDeviceParams() @@ -239,12 +245,12 @@ func (m *Manager) Add(options ...types.DeviceOption) (*models.Device, error) { return restResponse.Payload, nil } -// UpdateAndReplaceByID implements types.DeviceManager. -func (m *Manager) UpdateAndReplaceByID(id int32, options ...types.DeviceOption) (*models.Device, error) { - device := buildDevice(m.Config(), m.ControllerClient, options...) +// UpdateAndReplace implements types.DeviceManager. +func (m *Manager) UpdateAndReplace(d *models.Device, options ...types.DeviceOption) (*models.Device, error) { + device := buildDevice(m.Config(), m.ControllerClient, d, options...) log.Debugf("%#v", device) - return m.updateAndReplace(id, device) + return m.updateAndReplace(d.ID, device) } // UpdateAndReplaceByDisplayName implements types.DeviceManager. @@ -260,7 +266,7 @@ func (m *Manager) UpdateAndReplaceByDisplayName(name string, options ...types.De } options = append(options, m.DisplayName(*d.DisplayName)) // Update the device. - device, err := m.UpdateAndReplaceByID(d.ID, options...) + device, err := m.UpdateAndReplace(d, options...) if err != nil { return nil, err } @@ -270,13 +276,13 @@ func (m *Manager) UpdateAndReplaceByDisplayName(name string, options ...types.De // TODO: this method needs to be removed in DEV-50496 -// UpdateAndReplaceFieldByID implements types.DeviceManager. -func (m *Manager) UpdateAndReplaceFieldByID(id int32, field string, options ...types.DeviceOption) (*models.Device, error) { - device := buildDevice(m.Config(), m.ControllerClient, options...) +// UpdateAndReplaceField implements types.DeviceManager. +func (m *Manager) UpdateAndReplaceField(d *models.Device, field string, options ...types.DeviceOption) (*models.Device, error) { + device := buildDevice(m.Config(), m.ControllerClient, d, options...) log.Debugf("%#v", device) params := lm.NewPatchDeviceParams() - params.SetID(id) + params.SetID(d.ID) params.SetBody(device) opType := "replace" params.SetOpType(&opType) @@ -304,7 +310,7 @@ func (m *Manager) UpdateAndReplaceFieldByDisplayName(name string, field string, } options = append(options, m.DisplayName(*d.DisplayName)) // Update the device. - device, err := m.UpdateAndReplaceFieldByID(d.ID, field, options...) + device, err := m.UpdateAndReplaceField(d, field, options...) if err != nil { return nil, err } diff --git a/pkg/etcd/etcd.go b/pkg/etcd/etcd.go index 1e1480d05..7cfc23995 100644 --- a/pkg/etcd/etcd.go +++ b/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()), } } diff --git a/pkg/types/types.go b/pkg/types/types.go index 7b5717182..7a7cd2977 100644 --- a/pkg/types/types.go +++ b/pkg/types/types.go @@ -47,13 +47,13 @@ type DeviceMapper interface { FindByDisplayNameAndClusterName(string) (*models.Device, error) // Add adds a device to a LogicMonitor account. Add(...DeviceOption) (*models.Device, error) - // UpdateAndReplaceByID updates a device using the 'replace' OpType. - UpdateAndReplaceByID(int32, ...DeviceOption) (*models.Device, error) + // UpdateAndReplace updates a device using the 'replace' OpType. + UpdateAndReplace(*models.Device, ...DeviceOption) (*models.Device, error) // UpdateAndReplaceByDisplayName updates a device using the 'replace' OpType if and onlt if it does not already exist. UpdateAndReplaceByDisplayName(string, ...DeviceOption) (*models.Device, error) - // UpdateAndReplaceFieldByID updates a device using the 'replace' OpType for a + // UpdateAndReplaceField updates a device using the 'replace' OpType for a // specific field of a device. - UpdateAndReplaceFieldByID(int32, string, ...DeviceOption) (*models.Device, error) + UpdateAndReplaceField(*models.Device, string, ...DeviceOption) (*models.Device, error) // UpdateAndReplaceFieldByDisplayName updates a device using the 'replace' OpType for a // specific field of a device. UpdateAndReplaceFieldByDisplayName(string, string, ...DeviceOption) (*models.Device, error) diff --git a/pkg/utilities/utilities.go b/pkg/utilities/utilities.go index fb82d2c2d..6aa5658fc 100644 --- a/pkg/utilities/utilities.go +++ b/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 { diff --git a/pkg/watch/deployment/deployment.go b/pkg/watch/deployment/deployment.go index 273ce9d9f..cd144f088 100644 --- a/pkg/watch/deployment/deployment.go +++ b/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), diff --git a/pkg/watch/node/node.go b/pkg/watch/node/node.go index 4a139243b..0418741c0 100644 --- a/pkg/watch/node/node.go +++ b/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)), diff --git a/pkg/watch/pod/pod.go b/pkg/watch/pod/pod.go index d687542de..30f6ab95c 100644 --- a/pkg/watch/pod/pod.go +++ b/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), diff --git a/pkg/watch/service/service.go b/pkg/watch/service/service.go index 24902b4ef..5bdbbfcfb 100644 --- a/pkg/watch/service/service.go +++ b/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),