config: improved garbage collector options

So far the garbage collector only had one simple option that was to
delete images older than X days.

In this patch we are introducing a new option and a new behavior. User
will now be able to keep the latest N images through `keep_latest`
option. And the images won't be deleted even if older than X days if
they were pulled in the latest X days.

Signed-off-by: Vítor Avelino <vavelino@suse.com>

app/models/repository.rb

@@ -74,6 +74,17 @@ def delete_by!(actor)
     destroyed
   end
 
+  # Handle a pull event from the registry.
+  def self.handle_pull_event(event)
+    registry = Registry.find_from_event(event)
+    namespace, repo_name, tag_name = registry.get_namespace_from_event(event)
+    namespace = Namespace.global if namespace.nil?
+
+    repository = namespace.repositories.find_by(name: repo_name)
+    tag = repository.tags.find_by(name: tag_name)
+    tag.update_columns(pulled_at: Time.current)
+  end
+
   # Handle a push event from the registry.
   def self.handle_push_event(event)
     registry = Registry.find_from_event(event)

app/models/tag.rb

@@ -16,6 +16,7 @@
 #  username      :string(255)
 #  scanned       :integer          default(0)
 #  size          :integer
+#  pulled_at     :datetime
 #
 # Indexes
 #

app/models/webhook.rb

@@ -87,6 +87,9 @@ def self.handle_push_event(event)
     hydra.run
   end
 
+  # Pull event is not handled nu Webhook yet
+  def self.handle_pull_event(event); end
+
   # Handle a delete event from the registry. All enabled webhooks of the provided
   # namespace are triggered in parallel.
   def self.handle_delete_event(event)

config/config.yml

@@ -63,10 +63,17 @@ delete:
   garbage_collector:
     enabled: false
 
-    # Remove images older than a specific value. This value is interpreted as
-    # the number of days.
+    # Remove images not pulled and older than a specific value. This value is
+    # interpreted as the number of days.
+    #
+    # e.g.: If an image wasn't pulled in the latest 30 days and the image wasn't
+    # updated somehow in the latest 30 days, the image will be deleted.
     older_than: 30
 
+    # Keep the latest X images regardless if it's older than the value set in
+    # `older_than` configuration
+    keep_latest: 5
+
     # Provide a string containing a regular expression. If you provide a
     # valid regular expression, garbage collector will only be applied into tags
     # matching a given name.

db/migrate/20190115133935_add_pulled_at_to_tags.rb

@@ -0,0 +1,5 @@
+class AddPulledAtToTags < ActiveRecord::Migration[5.2]
+  def change
+    add_column :tags, :pulled_at, :datetime, default: nil
+  end
+end

db/schema.mysql.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 2019_01_09_112643) do
+ActiveRecord::Schema.define(version: 2019_01_15_133935) do
 
   create_table "activities", id: :integer, options: "ENGINE=InnoDB DEFAULT CHARSET=utf8", force: :cascade do |t|
     t.string "trackable_type"
@@ -121,6 +121,7 @@
     t.string "username"
     t.integer "scanned", default: 0
     t.integer "size"
+    t.datetime "pulled_at"
     t.index ["repository_id"], name: "index_tags_on_repository_id"
     t.index ["user_id"], name: "index_tags_on_user_id"
   end

db/schema.postgresql.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 2019_01_09_112643) do
+ActiveRecord::Schema.define(version: 2019_01_15_133935) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -124,6 +124,7 @@
     t.string "username"
     t.integer "scanned", default: 0
     t.integer "size"
+    t.datetime "pulled_at"
     t.index ["repository_id"], name: "index_tags_on_repository_id"
     t.index ["user_id"], name: "index_tags_on_user_id"
   end

lib/portus/background/garbage_collector.rb

@@ -29,6 +29,8 @@ def disable?
       end
 
       def execute!
+        return if Tag.all.count <= APP_CONFIG["delete"]["garbage_collector"]["keep_latest"].to_i
+
         @tags ||= tags_to_be_collected
         service = ::Tags::DestroyService.new(User.find_by(username: "portus"))
 
@@ -46,7 +48,10 @@ def to_s
       protected
 
       def tags_to_be_collected
-        tags = Tag.where(marked: false).where("updated_at < ?", older_than)
+        tags = Tag.where(marked: false)
+                  .where("updated_at < ? AND (pulled_at < ? OR pulled_at IS NULL)",
+                         older_than,
+                         older_than)
         return tags if APP_CONFIG["delete"]["garbage_collector"]["tag"].blank?
 
         rx = tag_regexp

lib/portus/registry_notification.rb

@@ -5,7 +5,7 @@ module Portus
   # consumed later on.
   class RegistryNotification
     # An array with the events that a handler has to support.
-    HANDLED_EVENTS = %w[push delete].freeze
+    HANDLED_EVENTS = %w[push delete pull].freeze
 
     # It filters the event from the registry so the background job can actually
     # handle this request.

spec/lib/portus/background/garbage_collector_spec.rb

@@ -1,11 +1,15 @@
 # frozen_string_literal: true
 
+require "rails_helper"
+require "portus/background/garbage_collector"
+
 describe ::Portus::Background::GarbageCollector do
   let(:old_tag)    { (APP_CONFIG["delete"]["garbage_collector"]["older_than"].to_i + 10).days.ago }
   let(:recent_tag) { (APP_CONFIG["delete"]["garbage_collector"]["older_than"].to_i - 10).days.ago }
 
   before do
     APP_CONFIG["delete"]["garbage_collector"]["enabled"] = true
+    APP_CONFIG["delete"]["garbage_collector"]["keep_latest"] = 0
   end
 
   it "returns the proper value for sleep_value" do
@@ -62,6 +66,12 @@
       expect(tags).to be_empty
     end
 
+    it "ignores older tags if pulled recently" do
+      create(:tag, name: "tag", repository: repository, updated_at: old_tag, pulled_at: recent_tag)
+      tags = subject.send(:tags_to_be_collected)
+      expect(tags).to be_empty
+    end
+
     it "exists a tag but it's considered recent" do
       create(:tag, name: "tag", repository: repository, updated_at: recent_tag)
       tags = subject.send(:tags_to_be_collected)
@@ -113,6 +123,19 @@
       end.to(change { Tag.all.count }.from(1).to(0))
     end
 
+    it "skips older tags if number of tags < keep_latest" do
+      APP_CONFIG["delete"]["garbage_collector"]["keep_latest"] = 5
+      create_list(:tag, 4, repository: repository, updated_at: old_tag)
+
+      expect { subject.execute! }.not_to change(Tag.all, :count)
+    end
+
+    it "skips older tags if it was pulled recently" do
+      create_list(:tag, 4, repository: repository, updated_at: old_tag, pulled_at: recent_tag)
+
+      expect { subject.execute! }.not_to change(Tag.all, :count)
+    end
+
     it "skips tags which could not be removed for whatever reason" do
       allow_any_instance_of(Tag).to(
         receive(:fetch_digest) { |tag| tag.digest == "wrong" ? "" : tag.digest }

spec/lib/portus/registry_notification_spec.rb

@@ -7,15 +7,16 @@
   let(:relevant)  { ::Portus::Fixtures::RegistryEvent::RELEVANT.dup }
   let(:delete)    { ::Portus::Fixtures::RegistryEvent::DELETE.dup }
   let(:version23) { ::Portus::Fixtures::RegistryEvent::VERSION23.dup }
+  let(:pull)      { ::Portus::Fixtures::RegistryEvent::PULL.dup }
 
   it "processes all the relevant events" do
-    evaluated_events = [relevant, delete, version23]
+    evaluated_events = [relevant, delete, version23, pull]
     evaluated_events.each { |e| body["events"] << e }
 
     described_class.process!(body)
 
     events = RegistryEvent.order(:event_id)
-    expect(events.size).to eq 3
+    expect(events.size).to eq 4
     events.each_with_index do |e, idx|
       data = JSON.parse(e.data)
 

spec/models/repository_spec.rb

@@ -71,6 +71,17 @@ def get_url(repo, tag)
     end
   end
 
+  describe "handle pull event" do
+    let(:tag_name) { "latest" }
+    let(:repository_name) { "busybox" }
+    let(:registry) do
+      create(:registry,
+             hostname:          "registry.test.lan",
+             external_hostname: "external.test.lan")
+    end
+    let(:user) { create(:user) }
+  end
+
   describe "handle push event" do
     let(:tag_name) { "latest" }
     let(:repository_name) { "busybox" }

spec/models/tag_spec.rb

@@ -16,6 +16,7 @@
 #  username      :string(255)
 #  scanned       :integer          default(0)
 #  size          :integer
+#  pulled_at     :datetime
 #
 # Indexes
 #

spec/support/registry_events.rb

@@ -11,14 +11,40 @@ module RegistryEvent
           "events" => [
             { "action" => "push" },
             { "action" => "push", "target" => { "mediaType" => "some" } },
-            {
-              "action" => "pull", "target" => {
-                "mediaType" => "application/vnd.docker.distribution.manifest.v1+json"
-              }
-            }
+            { "action" => "irrelevant", "target" => { "mediaType" => "some" } },
           ]
         }.freeze
 
+      PULL =
+        {
+          "id"        => "847f45bb-5f19-4c1b-b198-6c5ba467c127",
+          "timestamp" => "2019-01-15T20:17:10.595087128Z",
+          "action"    => "pull",
+          "target"    => {
+            "mediaType"  => "application/vnd.docker.distribution.manifest.v2+json",
+            "size"       => 2193,
+            "digest"     => "sha256:095ca87493f6a2147b8543a669f2d773094c3e38d675c27df9be7e17a981033c",
+            "length"     => 2193,
+            "repository" => "vitoravelino/etcd",
+            "url"        => "http://172.17.0.1:5000/v2/vitoravelino/etcd/manifests/sha256:095ca87493f6a2147b8543a669f2d773094c3e38d675c27df9be7e17a981033c",
+            "tag" => "v3.2.25-arm64"
+          },
+          "request" => {
+            "id"        => "e8904890-d30f-428d-b52c-4cb74e7707f9",
+            "addr"      => "172.19.0.1:54288",
+            "host"      => "172.17.0.1:5000",
+            "method"    => "GET",
+            "useragent" => "docker/18.09.0 go/go1.10.7 git-commit/e68fc7a215d7 kernel/4.19.12-1-default os/linux arch/amd64 UpstreamClient(Docker-Client/18.09.0 \\(linux\\))"
+          },
+          "actor" => {
+            "name" => "vitoravelino"
+          },
+          "source" => {
+            "addr"       => "50549da63cc2:5000",
+            "instanceID" => "a481f8c8-a71c-4395-b90c-f8d32a083d02"
+          }
+        }.freeze
+
       RELEVANT =
         {
           "id"     => "5d673710-06b5-48b5-a7d9-94cbaacf776b",