From f54e3e3e5a3f63678057c07976f797684406e1f3 Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Wed, 17 Jan 2024 12:12:10 +0200 Subject: [PATCH 1/4] Add metric for disk info & last update Signed-off-by: Danny Kopping --- aws/provider.go | 11 ++++++- aws/provider_test.go | 6 ++-- cmd/internal/providers.go | 2 +- cmd/unused-exporter/exporter.go | 31 ++++++++++++++++++ meta.go | 58 +++++++++++++++++++++++++++++++++ 5 files changed, 103 insertions(+), 5 deletions(-) diff --git a/aws/provider.go b/aws/provider.go index e1649ec..7e2eb6f 100644 --- a/aws/provider.go +++ b/aws/provider.go @@ -8,6 +8,7 @@ import ( "github.com/aws/aws-sdk-go-v2/service/ec2" "github.com/aws/aws-sdk-go-v2/service/ec2/types" "github.com/grafana/unused" + "github.com/inkel/logfmt" ) var _ unused.Provider = &Provider{} @@ -16,6 +17,7 @@ var _ unused.Provider = &Provider{} type Provider struct { client *ec2.Client meta unused.Meta + logger *logfmt.Logger } // Name returns AWS. @@ -29,7 +31,7 @@ func (p *Provider) Meta() unused.Meta { return p.meta } // A valid EC2 client must be supplied in order to list the unused // resources. The metadata passed will be used to identify the // provider. -func NewProvider(client *ec2.Client, meta unused.Meta) (*Provider, error) { +func NewProvider(logger *logfmt.Logger, client *ec2.Client, meta unused.Meta) (*Provider, error) { if meta == nil { meta = make(unused.Meta) } @@ -37,6 +39,7 @@ func NewProvider(client *ec2.Client, meta unused.Meta) (*Provider, error) { return &Provider{ client: client, meta: meta, + logger: logger, }, nil } @@ -45,10 +48,16 @@ func NewProvider(client *ec2.Client, meta unused.Meta) (*Provider, error) { func (p *Provider) ListUnusedDisks(ctx context.Context) (unused.Disks, error) { params := &ec2.DescribeVolumesInput{ Filters: []types.Filter{ + // only show available (i.e. not "in-use") volumes { Name: aws.String("status"), Values: []string{string(types.VolumeStateAvailable)}, }, + // exclude snapshots + { + Name: aws.String("snapshot-id"), + Values: []string{""}, + }, }, } diff --git a/aws/provider_test.go b/aws/provider_test.go index 94a7d4c..3ff3f05 100644 --- a/aws/provider_test.go +++ b/aws/provider_test.go @@ -22,7 +22,7 @@ func TestNewProvider(t *testing.T) { t.Fatalf("cannot load AWS config: %v", err) } - p, err := aws.NewProvider(ec2.NewFromConfig(cfg), nil) + p, err := aws.NewProvider(nil, ec2.NewFromConfig(cfg), nil) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -40,7 +40,7 @@ func TestProviderMeta(t *testing.T) { t.Fatalf("cannot load AWS config: %v", err) } - return aws.NewProvider(ec2.NewFromConfig(cfg), meta) + return aws.NewProvider(nil, ec2.NewFromConfig(cfg), meta) }) if err != nil { t.Fatalf("unexpected error: %v", err) @@ -121,7 +121,7 @@ func TestListUnusedDisks(t *testing.T) { t.Fatalf("cannot load AWS config: %v", err) } - p, err := aws.NewProvider(ec2.NewFromConfig(cfg), nil) + p, err := aws.NewProvider(nil, ec2.NewFromConfig(cfg), nil) if err != nil { t.Fatalf("unexpected error: %v", err) } diff --git a/cmd/internal/providers.go b/cmd/internal/providers.go index b9c5a44..e5363f9 100644 --- a/cmd/internal/providers.go +++ b/cmd/internal/providers.go @@ -41,7 +41,7 @@ func CreateProviders(ctx context.Context, logger *logfmt.Logger, gcpProjects, aw return nil, fmt.Errorf("cannot load AWS config for profile %s: %w", profile, err) } - p, err := aws.NewProvider(ec2.NewFromConfig(cfg), map[string]string{"profile": profile}) + p, err := aws.NewProvider(logger, ec2.NewFromConfig(cfg), map[string]string{"profile": profile}) if err != nil { return nil, fmt.Errorf("creating AWS provider for profile %s: %w", profile, err) } diff --git a/cmd/unused-exporter/exporter.go b/cmd/unused-exporter/exporter.go index 3a2a17d..6cb16c0 100644 --- a/cmd/unused-exporter/exporter.go +++ b/cmd/unused-exporter/exporter.go @@ -25,6 +25,7 @@ type exporter struct { count *prometheus.Desc dur *prometheus.Desc suc *prometheus.Desc + dlu *prometheus.Desc } func registerExporter(ctx context.Context, providers []unused.Provider, cfg config) error { @@ -59,6 +60,12 @@ func registerExporter(ctx context.Context, providers []unused.Provider, cfg conf "Static metric indicating if collecting the metrics succeeded or not", labels, nil), + + dlu: prometheus.NewDesc( + prometheus.BuildFQName(namespace, "disks", "last_used_at"), + "Kubernetes metadata associated with each unused disk, with the value as the last time the disk was used (if available)", + append(labels, []string{"disk", "created_for_pv", "created_for_pvc", "zone"}...), + nil), } return prometheus.Register(e) @@ -68,6 +75,7 @@ func (e *exporter) Describe(ch chan<- *prometheus.Desc) { ch <- e.info ch <- e.count ch <- e.dur + ch <- e.dlu } func (e *exporter) Collect(ch chan<- prometheus.Metric) { @@ -138,6 +146,29 @@ func (e *exporter) Collect(ch chan<- prometheus.Metric) { for ns, c := range countByNamespace { ch <- prometheus.MustNewConstMetric(e.count, prometheus.GaugeValue, float64(c), name, pid, ns) } + + for _, d := range disks { + m := d.Meta() + + var ts float64 + lastUsed := d.LastUsedAt() + if lastUsed.IsZero() { + ts = 0.0 + } else { + ts = float64(lastUsed.UnixMilli()) + } + + if m.CreatedForPV() == "" { + continue + } + + ch <- prometheus.MustNewConstMetric(e.dlu, prometheus.GaugeValue, ts, name, pid, + d.ID(), + m.CreatedForPV(), + m.CreatedForPVC(), + m.Zone(), + ) + } }(p) } diff --git a/meta.go b/meta.go index 0ee0ade..b48a6ac 100644 --- a/meta.go +++ b/meta.go @@ -32,8 +32,66 @@ func (m Meta) String() string { return s.String() } +func (m Meta) Equals(b Meta) bool { + akeys := m.Keys() + bkeys := b.Keys() + + if len(akeys) != len(bkeys) { + return false + } + + sort.Strings(akeys) + sort.Strings(bkeys) + + for ak, av := range m { + bv, ok := b[ak] + if !ok { + return false + } + + if av != bv { + return false + } + } + + return true +} + // Matches returns true when the given key exists in the map with the // given value. func (m Meta) Matches(key, val string) bool { return m[key] == val } + +func (m Meta) CreatedForPV() string { + return m.coalesce("kubernetes.io/created-for/pv/name", "kubernetes.io-created-for-pv-name") +} + +func (m Meta) CreatedForPVC() string { + return m.coalesce("kubernetes.io/created-for/pvc/name", "kubernetes.io-created-for-pvc-name") +} + +func (m Meta) CreatedForNamespace() string { + return m.coalesce("kubernetes.io/created-for/pvc/namespace", "kubernetes.io-created-for-pvc-namespace") +} + +func (m Meta) CreatedBy() string { + return m.coalesce("storage.gke.io/created-by", "created-by") +} + +func (m Meta) Zone() string { + return m.coalesce("zone", "location") +} + +func (m Meta) coalesce(keys ...string) string { + for _, k := range keys { + v, ok := m[k] + if !ok { + continue + } + + return v + } + + return "" +} From 8e3b74422ecbe51e3b6793c54e62b173d5dca162 Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Wed, 17 Jan 2024 13:01:05 +0200 Subject: [PATCH 2/4] Use helper method Signed-off-by: Danny Kopping --- cmd/unused-exporter/exporter.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/unused-exporter/exporter.go b/cmd/unused-exporter/exporter.go index 6cb16c0..613bfef 100644 --- a/cmd/unused-exporter/exporter.go +++ b/cmd/unused-exporter/exporter.go @@ -141,7 +141,7 @@ func (e *exporter) Collect(ch chan<- prometheus.Metric) { labels[k] = meta[k] } e.logger.Log("unused disk found", labels) - countByNamespace[meta["kubernetes.io/created-for/pvc/namespace"]] += 1 + countByNamespace[meta.CreatedForNamespace()] += 1 } for ns, c := range countByNamespace { ch <- prometheus.MustNewConstMetric(e.count, prometheus.GaugeValue, float64(c), name, pid, ns) From e6be5c8193c2a8fedf21aefd01afc335fcdf102f Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Fri, 19 Jan 2024 15:52:46 +0200 Subject: [PATCH 3/4] Update cmd/unused-exporter/exporter.go Co-authored-by: Mark --- cmd/unused-exporter/exporter.go | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/cmd/unused-exporter/exporter.go b/cmd/unused-exporter/exporter.go index 613bfef..b90f7d9 100644 --- a/cmd/unused-exporter/exporter.go +++ b/cmd/unused-exporter/exporter.go @@ -150,13 +150,11 @@ func (e *exporter) Collect(ch chan<- prometheus.Metric) { for _, d := range disks { m := d.Meta() - var ts float64 - lastUsed := d.LastUsedAt() - if lastUsed.IsZero() { - ts = 0.0 - } else { - ts = float64(lastUsed.UnixMilli()) - } +ts := 0.0 +lastUsed := d.LastUsedAt() +if !lastUsed.IsZero() { + ts = float64(lastUsed.UnixMilli()) +} if m.CreatedForPV() == "" { continue From 582d6e3e5a61dc5d0e2fef1ad6fa383d101e154b Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Mon, 22 Jan 2024 11:55:17 +0200 Subject: [PATCH 4/4] Adding tests, addressing review feedback Also changed test package so unexported functions could be tested Signed-off-by: Danny Kopping --- cmd/unused-exporter/exporter.go | 10 +-- meta.go | 14 +--- meta_test.go | 132 ++++++++++++++++++++++++++++++-- 3 files changed, 134 insertions(+), 22 deletions(-) diff --git a/cmd/unused-exporter/exporter.go b/cmd/unused-exporter/exporter.go index b90f7d9..d54f252 100644 --- a/cmd/unused-exporter/exporter.go +++ b/cmd/unused-exporter/exporter.go @@ -150,11 +150,11 @@ func (e *exporter) Collect(ch chan<- prometheus.Metric) { for _, d := range disks { m := d.Meta() -ts := 0.0 -lastUsed := d.LastUsedAt() -if !lastUsed.IsZero() { - ts = float64(lastUsed.UnixMilli()) -} + var ts float64 + lastUsed := d.LastUsedAt() + if !lastUsed.IsZero() { + ts = float64(lastUsed.UnixMilli()) + } if m.CreatedForPV() == "" { continue diff --git a/meta.go b/meta.go index b48a6ac..7aa42aa 100644 --- a/meta.go +++ b/meta.go @@ -33,23 +33,13 @@ func (m Meta) String() string { } func (m Meta) Equals(b Meta) bool { - akeys := m.Keys() - bkeys := b.Keys() - - if len(akeys) != len(bkeys) { + if len(m) != len(b) { return false } - sort.Strings(akeys) - sort.Strings(bkeys) - for ak, av := range m { bv, ok := b[ak] - if !ok { - return false - } - - if av != bv { + if !ok || av != bv { return false } } diff --git a/meta_test.go b/meta_test.go index 116e9b8..9258933 100644 --- a/meta_test.go +++ b/meta_test.go @@ -1,14 +1,12 @@ -package unused_test +package unused import ( "sort" "testing" - - "github.com/grafana/unused" ) func TestMeta(t *testing.T) { - m := &unused.Meta{ + m := &Meta{ "def": "123", "ghi": "456", "abc": "789", @@ -35,7 +33,7 @@ func TestMeta(t *testing.T) { } func TestMetaMatches(t *testing.T) { - m := &unused.Meta{ + m := &Meta{ "def": "123", "ghi": "456", "abc": "789", @@ -51,3 +49,127 @@ func TestMetaMatches(t *testing.T) { t.Error("expecting no match for different value") } } + +func TestCoalesce(t *testing.T) { + tests := []struct { + name string + m Meta + input []string + expected string + }{ + { + name: "single key returns self", + m: Meta{ + "foo": "bar", + }, + input: []string{"foo"}, + expected: "bar", + }, + { + name: "multiple keys returns first non-nil, single match", + m: Meta{ + "foo": "bar", + }, + input: []string{"buz", "foo"}, + expected: "bar", + }, + { + name: "multiple keys returns first non-nil, many possible matches", + m: Meta{ + "foo": "bar", + "buz": "qux", + }, + input: []string{"buz", "foo"}, + expected: "qux", + }, + { + name: "any value is returned if key is present", + m: Meta{ + "foo": "", + "buz": "qux", + }, + input: []string{"foo", "buz"}, + expected: "", + }, + { + name: "no given keys returns zero value", + m: Meta{ + "foo": "bar", + "buz": "qux", + }, + input: []string{}, + expected: "", + }, + { + name: "no matching keys returns zero value", + m: Meta{ + "foo": "bar", + "buz": "qux", + }, + input: []string{"nope"}, + expected: "", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + actual := tt.m.coalesce(tt.input...) + if tt.expected != actual { + t.Fatalf("expected %v but got %v", tt.expected, actual) + } + }) + } +} + +func TestEquals(t *testing.T) { + tests := []struct { + name string + m Meta + input Meta + expected bool + }{ + { + name: "nil values are equal", + m: Meta{}, + input: Meta{}, + expected: true, + }, + { + name: "nil & non-nil values are not equal", + m: Meta{"not": "nil"}, + input: Meta{}, + expected: false, + }, + { + name: "same keys but different values are not equal", + m: Meta{"a": "b"}, + input: Meta{"a": "c"}, + expected: false, + }, + { + name: "same values but different keys are not equal", + m: Meta{"a": "b"}, + input: Meta{"c": "b"}, + expected: false, + }, + { + name: "same keys & values are equal", + m: Meta{"a": "b", "c": "d"}, + input: Meta{"a": "b", "c": "d"}, + expected: true, + }, + { + name: "order is irrelevant", + m: Meta{"a": "b", "c": "d"}, + input: Meta{"c": "d", "a": "b"}, + expected: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + actual := tt.m.Equals(tt.input) + if tt.expected != actual { + t.Fatalf("expected %v but got %v", tt.expected, actual) + } + }) + } +}