Implement Loki curator #85

Kristian-ZH · 2021-01-25T11:07:52Z

How to categorize this PR?

/kind enhancement
/area logging
/priority normal

What this PR does / why we need it:
Add new Loki curator which will ensure that Loki's inodes and Storage limits are not reached.
The curator is entirely configured from a configuration file whose path is provided via curator's config flag.

Which issue(s) this PR fixes:
#72

Special notes for your reviewer:
@vlvasilev

Release note:

Loki curator will ensure that Loki's Inods and Storage limits are not reached

vpnachev

Some comments as first round of reviews.

Dockerfile

Makefile

cmd/loki-curator/app/loki_curator.go

pkg/loki-curator/config/config.go

pkg/loki-curator/config/config_test.go

pkg/loki-curator/disk-curator.go

Kristian-ZH · 2021-01-28T07:27:48Z

Thanks for the comments, will address them today

Dockerfile

VERSION

go.mod

cmd/loki-curator/main.go

vpnachev

Do not push build artifacts to the repository, I mean the file build/curator

pkg/loki-curator/config/config.go

pkg/loki-curator/disk-curator.go

pkg/loki-curator/curator.go

pkg/loki-curator/inode-curator.go

vpnachev

Some minor nits.

Makefile

pkg/loki/curator/config/config.go

pkg/loki/curator/config/config_suite_test.go

pkg/loki/curator/config/config.go

pkg/loki/curator/curator.go

pkg/loki/curator/disk.go

pkg/loki/curator/utils/utils.go

pkg/loki/curator/utils/utils_suite_test.go

pkg/loki/curator/utils/utils_test.go

vpnachev

Please, update also the .gitignore file together with this PR and remove the binary build/curator.
You can see good examples how to do it gardener-attic/gardenctl#488 and gardener-attic/gardenctl#491.

Kristian-ZH · 2021-02-08T11:17:26Z

/ping @vpnachev

gardener-robot · 2021-02-08T11:17:30Z

@vpnachev

Message

/ping @vpnachev

.gitignore

Dockerfile

cmd/loki-curator/app/loki_curator.go

cmd/loki-curator/main.go

vpnachev · 2021-02-08T12:11:25Z

pkg/loki/curator/config/config.go

+	MinFreePercentages             int `yaml:"MinFreePercentages,omitempty"`
+	TargetFreePercentages          int `yaml:"TargetFreePercentages,omitempty"`
+	PageSizeForDeletionPercentages int `yaml:"PageSizeForDeletionPercentages,omitempty"`


Hm, how is omitempty working with int?
It is better to use *int which allow to properly differ between unset value nil and zero value 0.

On the other side, I see these values are required, thus you can make them mandatory and keep them as int.

omitempty for int will check if the given field is not in its default state (e.g. 0 for int).

pkg/loki/curator/config/config.go

pkg/loki/curator/disk.go

vpnachev · 2021-02-08T12:50:16Z

/author-action

gardener-robot · 2021-02-08T12:50:26Z

@Kristian-ZH The pull request was assigned to you under author-action. Please unassign yourself when you are done. Thank you.

vpnachev

/lgtm

vlvasilev

/lgtm

vpnachev · 2021-02-09T15:44:26Z

Dockerfile

@@ -1,13 +1,23 @@
 #############      builder       #############
-FROM golang:1.14.2 AS builder
+FROM golang:1.15.7 AS builder


There is some other places where golang image is also used, e.g.

logging/.ci/pipeline_definitions

Line 23 in c4a3498

image: 'golang:1.14.2'

Kristian-ZH requested a review from vlvasilev as a code owner January 25, 2021 11:07

gardener-robot added area/logging Logging related kind/enhancement Enhancement, improvement, extension priority/normal needs/review Needs review size/xl Size of pull request is huge (see gardener-robot robot/bots/size.py) needs/second-opinion Needs second review by someone else labels Jan 25, 2021

gardener-robot-ci-1 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jan 25, 2021

Kristian-ZH force-pushed the add-curator branch from 416bebf to 1fa5933 Compare January 25, 2021 11:08

gardener-robot-ci-1 added needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Jan 25, 2021

vpnachev reviewed Jan 27, 2021

View reviewed changes

Kristian-ZH force-pushed the add-curator branch from 1fa5933 to 630748a Compare January 28, 2021 11:49

gardener-robot-ci-1 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jan 28, 2021

Kristian-ZH requested a review from vpnachev January 28, 2021 11:50

gardener-robot-ci-1 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jan 28, 2021

vlvasilev suggested changes Jan 28, 2021

View reviewed changes

Dockerfile Outdated Show resolved Hide resolved

VERSION Outdated Show resolved Hide resolved

go.mod Outdated Show resolved Hide resolved

cmd/loki-curator/main.go Outdated Show resolved Hide resolved

gardener-robot added the needs/changes Needs (more) changes label Jan 28, 2021

vpnachev reviewed Jan 29, 2021

View reviewed changes

pkg/loki-curator/config/config.go Outdated Show resolved Hide resolved

pkg/loki-curator/disk-curator.go Outdated Show resolved Hide resolved

pkg/loki-curator/curator.go Outdated Show resolved Hide resolved

pkg/loki-curator/inode-curator.go Outdated Show resolved Hide resolved

Kristian-ZH force-pushed the add-curator branch from 630748a to 906d0a7 Compare January 29, 2021 13:13

gardener-robot-ci-2 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jan 29, 2021

gardener-robot-ci-3 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jan 29, 2021

Kristian-ZH requested review from vpnachev and vlvasilev January 29, 2021 13:15

Kristian-ZH force-pushed the add-curator branch from 906d0a7 to feee2c3 Compare January 29, 2021 13:28

gardener-robot-ci-2 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jan 29, 2021

gardener-robot-ci-3 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jan 29, 2021

vpnachev reviewed Jan 29, 2021

View reviewed changes

vpnachev requested changes Jan 29, 2021

View reviewed changes

vpnachev reviewed Feb 8, 2021

View reviewed changes

gardener-robot added the status/author-action Issue requires issue author action label Feb 8, 2021

gardener-robot assigned Kristian-ZH Feb 8, 2021

Kristian-ZH force-pushed the add-curator branch from 18fe18a to 606ceb6 Compare February 9, 2021 07:56

gardener-robot-ci-2 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Feb 9, 2021

Kristian-ZH requested a review from vpnachev February 9, 2021 07:57

gardener-robot-ci-1 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Feb 9, 2021

Kristian-ZH removed the status/author-action Issue requires issue author action label Feb 9, 2021

vpnachev approved these changes Feb 9, 2021

View reviewed changes

gardener-robot added reviewed/lgtm Has approval for merging and removed needs/changes Needs (more) changes needs/review Needs review needs/second-opinion Needs second review by someone else labels Feb 9, 2021

Implement Loki curator

ad0f8b0

Kristian-ZH force-pushed the add-curator branch from 606ceb6 to ad0f8b0 Compare February 9, 2021 09:23

gardener-robot added needs/second-opinion Needs second review by someone else and removed reviewed/lgtm Has approval for merging labels Feb 9, 2021

gardener-robot-ci-2 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Feb 9, 2021

gardener-robot-ci-3 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Feb 9, 2021

vlvasilev approved these changes Feb 9, 2021

View reviewed changes

gardener-robot added reviewed/lgtm Has approval for merging and removed needs/second-opinion Needs second review by someone else labels Feb 9, 2021

Kristian-ZH merged commit fa26d3d into gardener:master Feb 9, 2021

vpnachev reviewed Feb 9, 2021

View reviewed changes

gardener-robot added priority/3 Priority (lower number equals higher priority) and removed priority/3 Priority (lower number equals higher priority) labels Mar 8, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement Loki curator #85

Implement Loki curator #85

Kristian-ZH commented Jan 25, 2021 •

edited

Loading

vpnachev left a comment

Kristian-ZH commented Jan 28, 2021

vpnachev left a comment

vpnachev left a comment

vpnachev left a comment

Kristian-ZH commented Feb 8, 2021

gardener-robot commented Feb 8, 2021

vpnachev Feb 8, 2021

Kristian-ZH Feb 8, 2021

vpnachev commented Feb 8, 2021

gardener-robot commented Feb 8, 2021

vpnachev left a comment

vlvasilev left a comment

vpnachev Feb 9, 2021

Implement Loki curator #85

Implement Loki curator #85

Conversation

Kristian-ZH commented Jan 25, 2021 • edited Loading

vpnachev left a comment

Choose a reason for hiding this comment

Kristian-ZH commented Jan 28, 2021

vpnachev left a comment

Choose a reason for hiding this comment

vpnachev left a comment

Choose a reason for hiding this comment

vpnachev left a comment

Choose a reason for hiding this comment

Kristian-ZH commented Feb 8, 2021

gardener-robot commented Feb 8, 2021

vpnachev Feb 8, 2021

Choose a reason for hiding this comment

Kristian-ZH Feb 8, 2021

Choose a reason for hiding this comment

vpnachev commented Feb 8, 2021

gardener-robot commented Feb 8, 2021

vpnachev left a comment

Choose a reason for hiding this comment

vlvasilev left a comment

Choose a reason for hiding this comment

vpnachev Feb 9, 2021

Choose a reason for hiding this comment

Kristian-ZH commented Jan 25, 2021 •

edited

Loading