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.

Sign up for GitHub

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

add a priority label #288

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

add a priority label #288

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 3 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,9 @@ enrich version checking on image tags:
is. In this example, the current version of `my-container` will be compared
against the image versions in the `docker.io/bitnami/etcd` registry.

- `priority.version-checker.io/my-container: 3`: will set the priority label.
So you can sort/prioritize better if there are many images in the cluster.

## Known configurations

From time to time, version-checker may need some of the above options applied to determine the latest version,
Expand Down
5 changes: 5 additions & 0 deletions pkg/api/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@ const (

// PinPatchAnnotationKey will pin the patch version to check.
PinPatchAnnotationKey = "pin-patch.version-checker.io"

// PriorityKey will set the priority label on the image metric.
PriorityKey = "priority.version-checker.io"
)

// Options is used to describe what restrictions should be used for determining
Expand All @@ -58,6 +61,8 @@ type Options struct {
PinPatch *int64 `json:"pin-patch,omitempty"`

RegexMatcher *regexp.Regexp `json:"-"`

Priority int
}

// ImageTag describes a container image tag.
Expand Down
14 changes: 11 additions & 3 deletions pkg/controller/checker/checker.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ type Result struct {
LatestVersion string
IsLatest bool
ImageURL string
Priority int
}

func New(search search.Searcher) *Checker {
Expand All @@ -48,11 +49,18 @@ func (c *Checker) Container(ctx context.Context, log *logrus.Entry, pod *corev1.

imageURL = c.overrideImageURL(log, imageURL, opts)

var result *Result
var err error
if opts.UseSHA {
return c.handleSHA(ctx, imageURL, statusSHA, opts, usingTag, currentTag)
result, err = c.handleSHA(ctx, imageURL, statusSHA, opts, usingTag, currentTag)
} else {
result, err = c.handleSemver(ctx, imageURL, statusSHA, currentTag, usingSHA, opts)
}

return c.handleSemver(ctx, imageURL, statusSHA, currentTag, usingSHA, opts)
if err != nil {
return result, err
}
result.Priority = opts.Priority
return result, err
}

func (c *Checker) handleLatestOrEmptyTag(log *logrus.Entry, currentTag, currentSHA string, opts *api.Options) {
Expand Down
11 changes: 11 additions & 0 deletions pkg/controller/options/options.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ func (b *Builder) Options(name string) (*api.Options, error) {
b.handlePinMinorOption,
b.handlePinPatchOption,
b.handleOverrideURLOption,
b.handlePriority,
}

// Execute each handler
Expand Down Expand Up @@ -146,6 +147,16 @@ func (b *Builder) handleOverrideURLOption(name string, opts *api.Options, setNon
return nil
}

func (b *Builder) handlePriority(name string, opts *api.Options, _ *bool, _ *[]string) error {
if priority, ok := b.ans[b.index(name, api.PriorityKey)]; ok {
var err error
if opts.Priority, err = strconv.Atoi(priority); err != nil {
return err
}
}
return nil
}

// IsEnabled will return whether the container has the enabled annotation set.
// Will fall back to default, if not set true/false.
func (b *Builder) IsEnabled(defaultEnabled bool, name string) bool {
Expand Down
2 changes: 2 additions & 0 deletions pkg/controller/sync.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package controller
import (
"context"
"fmt"
"strconv"
"strings"

"github.com/sirupsen/logrus"
Expand Down Expand Up @@ -97,6 +98,7 @@ func (c *Controller) checkContainer(ctx context.Context, log *logrus.Entry, pod
container.Name, containerType,
result.ImageURL, result.IsLatest,
result.CurrentVersion, result.LatestVersion,
strconv.Itoa(result.Priority),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as with metrics.go - I'm not entirely sure this would be needed.

)

return nil
Expand Down
9 changes: 5 additions & 4 deletions pkg/metrics/metrics.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ func New(log *logrus.Entry) *Metrics {
Help: "Where the container in use is using the latest upstream registry version",
},
[]string{
"namespace", "pod", "container", "container_type", "image", "current_version", "latest_version",
"namespace", "pod", "container", "container_type", "image", "current_version", "latest_version", "priority",
},
)

Expand Down Expand Up @@ -87,7 +87,7 @@ func (m *Metrics) Run(servingAddress string) error {
return nil
}

func (m *Metrics) AddImage(namespace, pod, container, containerType, imageURL string, isLatest bool, currentVersion, latestVersion string) {
func (m *Metrics) AddImage(namespace, pod, container, containerType, imageURL string, isLatest bool, currentVersion, latestVersion, priority string) {
// Remove old image url/version if it exists
m.RemoveImage(namespace, pod, container, containerType)

Expand All @@ -100,7 +100,7 @@ func (m *Metrics) AddImage(namespace, pod, container, containerType, imageURL st
}

m.containerImageVersion.With(
m.buildLabels(namespace, pod, container, containerType, imageURL, currentVersion, latestVersion),
m.buildLabels(namespace, pod, container, containerType, imageURL, currentVersion, latestVersion, priority),
).Set(isLatestF)

index := m.latestImageIndex(namespace, pod, container, containerType)
Expand Down Expand Up @@ -131,7 +131,7 @@ func (m *Metrics) latestImageIndex(namespace, pod, container, containerType stri
return strings.Join([]string{namespace, pod, container, containerType}, "")
}

func (m *Metrics) buildLabels(namespace, pod, container, containerType, imageURL, currentVersion, latestVersion string) prometheus.Labels {
func (m *Metrics) buildLabels(namespace, pod, container, containerType, imageURL, currentVersion, latestVersion, priority string) prometheus.Labels {
return prometheus.Labels{
"namespace": namespace,
"pod": pod,
Expand All @@ -140,6 +140,7 @@ func (m *Metrics) buildLabels(namespace, pod, container, containerType, imageURL
"image": imageURL,
"current_version": currentVersion,
"latest_version": latestVersion,
"priority": priority,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need priority to be output as a metric label? What is the intended use case of this? I'm concerned that this could increase cardinality unnecessarily? I'd favour logging over exposing as a metrics label.

}
}

Expand Down
10 changes: 7 additions & 3 deletions pkg/metrics/metrics_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,21 @@ import (
"github.com/sirupsen/logrus"
)

const (
defaultPriority = "0"
)

func TestCache(t *testing.T) {
m := New(logrus.NewEntry(logrus.New()))

for i, typ := range []string{"init", "container"} {
version := fmt.Sprintf("0.1.%d", i)
m.AddImage("namespace", "pod", "container", typ, "url", true, version, version)
m.AddImage("namespace", "pod", "container", typ, "url", true, version, version, defaultPriority)
}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we do need this, (see comment in metrics.go) Can we have a test that covers the setting and reflection of a different priority and validate that the metric set reflects the value given.

for i, typ := range []string{"init", "container"} {
version := fmt.Sprintf("0.1.%d", i)
mt, _ := m.containerImageVersion.GetMetricWith(m.buildLabels("namespace", "pod", "container", typ, "url", version, version))
mt, _ := m.containerImageVersion.GetMetricWith(m.buildLabels("namespace", "pod", "container", typ, "url", version, version, defaultPriority))
count := testutil.ToFloat64(mt)
if count != 1 {
t.Error("Should have added metric")
Expand All @@ -30,7 +34,7 @@ func TestCache(t *testing.T) {
}
for i, typ := range []string{"init", "container"} {
version := fmt.Sprintf("0.1.%d", i)
mt, _ := m.containerImageVersion.GetMetricWith(m.buildLabels("namespace", "pod", "container", typ, "url", version, version))
mt, _ := m.containerImageVersion.GetMetricWith(m.buildLabels("namespace", "pod", "container", typ, "url", version, version, defaultPriority))
count := testutil.ToFloat64(mt)
if count != 0 {
t.Error("Should have removed metric")
Expand Down