Add the possibility to disable background tasks #1631

termdew · 2018-01-25T13:04:23Z

Add possibility to disable background tasks

mssola · 2018-01-25T13:39:26Z

lib/portus/background/registry.rb

@@ -14,9 +14,13 @@ def sleep_value
      # options and because the `execute!` method will deal with valid rows
      # already.
      def work?
-        true


You should've left this as it was

mssola · 2018-01-25T13:40:20Z

lib/portus/background/registry.rb

      end

+			def enabled?
+			  return APP_CONFIG("background.registry")


Don't use return, ruby will implicitly return the last eval'd object. Moreover, it should be like this:

APP_CONFIG.enabled?("background.registry")

mssola · 2018-01-25T13:40:58Z

config/config.yml

+
+background:
+  registry: true
+  sync: true


Prefer the convention:

background: registry: enabled: true sync: true enabled: true

Plus, add documentation.

mssola · 2018-01-25T13:41:21Z

lib/portus/background/security_scanning.rb

@@ -17,6 +17,10 @@ def work?
        ::Portus::Security.enabled? && Tag.exists?(scanned: Tag.statuses[:scan_none])
      end

+			def enabled?
+			  return ::Portus::Security.enabled?


Remove the return.

mssola · 2018-01-25T13:41:34Z

lib/portus/background/sync.rb

@@ -18,6 +18,10 @@ def work?
        true
      end

+      def enabled?
+			  return APP_CONFIG("background.sync")


Same as with the registry

mssola

Besides my comments, could you provide tests for each enabled? method ?

mssola · 2018-01-29T15:25:20Z

lib/portus/background/registry.rb

+        else
+          APP_CONFIG.enabled?("background.registry")
+        end
+      end


I don't think APP_CONFIG("background.registry") will work. Do this instead:

def enabled? val = APP_CONFIG.enabled?("background.registry") Rails.logger.warn("WARNING: Registry is disabled!") unless val val end

After that, notice the following:

You don't have to start the message with WARNING, since the warn method will already cover that.

The message might not be clear to users. Instead, prefer something like: "Registry integration has been disabled. This is highly discouraged!". For the sync: "Registry synchronization has been disabled. This can lead into consistency problems!"

We have historically struggled with the synchronization of the Registry. This was developed at first because the following situation could happen: 1. User pushes image:tag. 2. Portus authorizes the transaction. 3. Registry sends the event to Portus with all the data, but Portus is unavailable. 4. Portus is up again but it missed the registry event. This is not the case anymore, because all the things that could make Portus unavailable have been either fixed or moved into the background process. Thus, this problem can be ignored in most cases. Moreover, this feature is quite dangerous because a bug on this code can make Portus wipe all the repositories, and historically "funny" issues have happened. This commit builds upon SUSE#1631 where background tasks can be disabled, and it disables the `sync` task by default. Moreover, it also adds a `sync-strategy` option which is available when users enable this feature. This option has four possible values: - `update-delete`: the same behavior we've had up until now. - `update`: similar to `update-delete` but this one will not delete repositories which no longer exist on the registry. This is useful for users which don't trust the risky `update-delete` strategy. - `on-start`: execute `update-delete` only once (on start). - `initial`: execute `update-delete` only once on start and only if the registry is empty. This is the default strategy since it might be convenient for new users that have already a running registry. This commit does not fix all the issues mentioned in SUSE#1631, but it hopefully does the trick for most users. Fixes SUSE#1650 Fixes SUSE#1664 See SUSE#1599 Depends on SUSE#1631 Signed-off-by: Miquel Sabaté Solà <[email protected]>

I have removed all the code that is deprecated in 2.3. Thus: - Removed `portusctl` in favor of openSUSE/portusctl. - Removed deprecated methods from lib/portus/migrate.rb. - Removed rake tasks that dealt with deprecated functionality (e.g. man pages for portusctl). Signed-off-by: Miquel Sabaté Solà <[email protected]>

Both Thor and md2man were used by the old `portusctl`, which has been removed. Signed-off-by: Miquel Sabaté Solà <[email protected]>

Signed-off-by: Miquel Sabaté Solà <[email protected]>

The version and the health endpoints needed an entity, so they could be better documented. Moreover, I've adapted some strings (e.g. removing dots), so it's better suited for efforts like SUSE#1645. Signed-off-by: Miquel Sabaté Solà <[email protected]>

Depends on SUSE#1645 Signed-off-by: Miquel Sabaté Solà <[email protected]>

Fixes SUSE#1632 Signed-off-by: Miquel Sabaté Solà <[email protected]>

There were some calls to present which were missing the type context. This should fix some oddities. Signed-off-by: Miquel Sabaté Solà <[email protected]>

closes SUSE#1652

Fixes SUSE#1654 Signed-off-by: Miquel Sabaté Solà <[email protected]>

This also clarifies some issues that we had with asset-related gems. See SUSE/velum#432 Fixes SUSE#1655 Signed-off-by: Miquel Sabaté Solà <[email protected]>

It will only remove `bundler`, which we need... Signed-off-by: Miquel Sabaté Solà <[email protected]>

Signed-off-by: Miquel Sabaté Solà <[email protected]>

After a security scanning iteration, some tags might not be in a :scan_done state. This is possible in situtations like: - Clair was unavailable for some time because of a hiccup. - There's an actual bug on the security scanning. In these cases, we will check in the end of the iteration whether all tags have been marked as scan. If this is not the case, then we will log a warning and mark the failed tags as :scan_none (so in the next iteration they can be picked up and hopefully be scanned properly this time). See SUSE#1649 Signed-off-by: Miquel Sabaté Solà <[email protected]>

Some images like `node:8` had so many vulnerabilities that then the stored JSON was huge. This is a temporary fix because we are about to release 2.3 and this fix is needed. A more suitable approach is the one mentioned in SUSE#1669. Fixes SUSE#1657 Signed-off-by: Miquel Sabaté Solà <[email protected]>

This commit adds two new endpoints: `/vulnerabilities` and `/vulnerabilities/:id`. Both these endpoints require admin privileges, and they will simply toggle the `scanned` column of tags so the scanning task can pick them up. This commit does not deal with the UI part, it simply provides the backend code. The frontend code can be delivered later on. See SUSE#1658 Signed-off-by: Miquel Sabaté Solà <[email protected]>

mssola · 2018-02-12T09:55:11Z

This has moved into #1679.

We have historically struggled with the synchronization of the Registry. This was developed at first because the following situation could happen: 1. User pushes image:tag. 2. Portus authorizes the transaction. 3. Registry sends the event to Portus with all the data, but Portus is unavailable. 4. Portus is up again but it missed the registry event. This is not the case anymore, because all the things that could make Portus unavailable have been either fixed or moved into the background process. Thus, this problem can be ignored in most cases. Moreover, this feature is quite dangerous because a bug on this code can make Portus wipe all the repositories, and historically "funny" issues have happened. This commit builds upon SUSE#1631 where background tasks can be disabled, and it disables the `sync` task by default. Moreover, it also adds a `sync-strategy` option which is available when users enable this feature. This option has four possible values: - `update-delete`: the same behavior we've had up until now. - `update`: similar to `update-delete` but this one will not delete repositories which no longer exist on the registry. This is useful for users which don't trust the risky `update-delete` strategy. - `on-start`: execute `update-delete` only once (on start). - `initial`: execute `update-delete` only once on start and only if the registry is empty. This is the default strategy since it might be convenient for new users that have already a running registry. This commit does not fix all the issues mentioned in SUSE#1631, but it hopefully does the trick for most users. Fixes SUSE#1650 Fixes SUSE#1664 See SUSE#1599 Depends on SUSE#1631 Signed-off-by: Miquel Sabaté Solà <[email protected]>

We have historically struggled with the synchronization of the Registry. This was developed at first because the following situation could happen: 1. User pushes image:tag. 2. Portus authorizes the transaction. 3. Registry sends the event to Portus with all the data, but Portus is unavailable. 4. Portus is up again but it missed the registry event. This is not the case anymore, because all the things that could make Portus unavailable have been either fixed or moved into the background process. Thus, this problem can be ignored in most cases. Moreover, this feature is quite dangerous because a bug on this code can make Portus wipe all the repositories, and historically "funny" issues have happened. This commit builds upon SUSE#1631 where background tasks can be disabled. Moreover, it also adds a `sync-strategy` option which is available when users enable this feature. This option has four possible values: - `update-delete`: the same behavior we've had up until now. - `update`: similar to `update-delete` but this one will not delete repositories which no longer exist on the registry. This is useful for users which don't trust the risky `update-delete` strategy. - `on-start`: execute `update-delete` only once (on start). - `initial`: execute `update-delete` only once on start and only if the registry is empty. This is the default strategy since it might be convenient for new users that have already a running registry. This commit does not fix all the issues mentioned in SUSE#1631, but it hopefully does the trick for most users. Fixes SUSE#1650 Fixes SUSE#1664 See SUSE#1599 Depends on SUSE#1631 Signed-off-by: Miquel Sabaté Solà <[email protected]>

We have historically struggled with the synchronization of the Registry. This was developed at first because the following situation could happen: 1. User pushes image:tag. 2. Portus authorizes the transaction. 3. Registry sends the event to Portus with all the data, but Portus is unavailable. 4. Portus is up again but it missed the registry event. This is not the case anymore, because all the things that could make Portus unavailable have been either fixed or moved into the background process. Thus, this problem can be ignored in most cases. Moreover, this feature is quite dangerous because a bug on this code can make Portus wipe all the repositories, and historically "funny" issues have happened. This commit builds upon SUSE#1631 where background tasks can be disabled. Moreover, it also adds a `sync-strategy` option which is available when users enable this feature. This option has four possible values: - `update-delete`: the same behavior we've had up until now. - `update`: similar to `update-delete` but this one will not delete repositories which no longer exist on the registry. This is useful for users which don't trust the risky `update-delete` strategy. - `on-start`: execute `update-delete` only once (on start). - `initial`: execute `update-delete` only once on start and only if the registry is empty. This is the default strategy since it might be convenient for new users that have already a running registry. This commit does not fix all the issues mentioned in SUSE#1599, but it hopefully does the trick for most users. Fixes SUSE#1650 Fixes SUSE#1664 See SUSE#1599 Depends on SUSE#1631 Signed-off-by: Miquel Sabaté Solà <[email protected]>

We have historically struggled with the synchronization of the Registry. This was developed at first because the following situation could happen: 1. User pushes image:tag. 2. Portus authorizes the transaction. 3. Registry sends the event to Portus with all the data, but Portus is unavailable. 4. Portus is up again but it missed the registry event. This is not the case anymore, because all the things that could make Portus unavailable have been either fixed or moved into the background process. Thus, this problem can be ignored in most cases. Moreover, this feature is quite dangerous because a bug on this code can make Portus wipe all the repositories, and historically "funny" issues have happened. This commit builds upon #1631 where background tasks can be disabled. Moreover, it also adds a `sync-strategy` option which is available when users enable this feature. This option has four possible values: - `update-delete`: the same behavior we've had up until now. - `update`: similar to `update-delete` but this one will not delete repositories which no longer exist on the registry. This is useful for users which don't trust the risky `update-delete` strategy. - `on-start`: execute `update-delete` only once (on start). - `initial`: execute `update-delete` only once on start and only if the registry is empty. This is the default strategy since it might be convenient for new users that have already a running registry. This commit does not fix all the issues mentioned in #1599, but it hopefully does the trick for most users. Fixes #1650 Fixes #1664 See #1599 Depends on #1631 Signed-off-by: Miquel Sabaté Solà <[email protected]>

mssola suggested changes Jan 25, 2018

View reviewed changes

termdew force-pushed the DisableBackgroundTasks branch 8 times, most recently from a568ef3 to e4242f1 Compare January 26, 2018 13:21

mssola suggested changes Jan 29, 2018

View reviewed changes

Vad1mo mentioned this pull request Feb 7, 2018

Background Process should not delete the whole registry and ad it again. #1664

Closed

mssola mentioned this pull request Feb 9, 2018

Stabilize the sync background task before the 2.3 release #1675

Merged

Fabian Baumanis and others added 16 commits February 12, 2018 09:30

Add the possibility to disable background tasks

41a3c98

Removed gems that are no longer needed

0be6db5

Both Thor and md2man were used by the old `portusctl`, which has been removed. Signed-off-by: Miquel Sabaté Solà <[email protected]>

packaging: added portusctl

781bdbe

Signed-off-by: Miquel Sabaté Solà <[email protected]>

Updated README file to new versions

df1cacd

Signed-off-by: Miquel Sabaté Solà <[email protected]>

packaging: fixed readme note on portusctl

53c8b49

Signed-off-by: Miquel Sabaté Solà <[email protected]>

help: point to the API documentation on production

41df18c

Depends on SUSE#1645 Signed-off-by: Miquel Sabaté Solà <[email protected]>

compose: fixed swagger's documentation URL

44df952

Fixes SUSE#1632 Signed-off-by: Miquel Sabaté Solà <[email protected]>

api: added the current type on some calls

6004306

There were some calls to present which were missing the type context. This should fix some oddities. Signed-off-by: Miquel Sabaté Solà <[email protected]>

rename swagger_doc into official naming openapi-spec.

f4f5bfd

closes SUSE#1652

js: added unit tests for vue components and utils

d038409

health: don't panic on malformed clair url

c61e015

Fixes SUSE#1654 Signed-off-by: Miquel Sabaté Solà <[email protected]>

packaging: add DB-related gems into the bundle

1237b22

This also clarifies some issues that we had with asset-related gems. See SUSE/velum#432 Fixes SUSE#1655 Signed-off-by: Miquel Sabaté Solà <[email protected]>

packaging: do not run bundle clean

4dde764

It will only remove `bundler`, which we need... Signed-off-by: Miquel Sabaté Solà <[email protected]>

api: added create and update methods to registries

5a16f20

Signed-off-by: Miquel Sabaté Solà <[email protected]>

mssola and others added 4 commits February 12, 2018 09:30

add new warning for disabled registry integration

5f88e65

termdew force-pushed the DisableBackgroundTasks branch from e4242f1 to 5f88e65 Compare February 12, 2018 09:24

termdew mentioned this pull request Feb 12, 2018

add the possibility to disable background tasks #1679

Merged

mssola closed this Feb 12, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add the possibility to disable background tasks #1631

Add the possibility to disable background tasks #1631

termdew commented Jan 25, 2018

mssola Jan 25, 2018

mssola Jan 25, 2018

mssola Jan 25, 2018

mssola Jan 25, 2018

mssola Jan 25, 2018

mssola Jan 25, 2018

mssola left a comment

mssola Jan 29, 2018

mssola commented Feb 12, 2018

Add the possibility to disable background tasks #1631

Add the possibility to disable background tasks #1631

Conversation

termdew commented Jan 25, 2018

mssola Jan 25, 2018

Choose a reason for hiding this comment

mssola Jan 25, 2018

Choose a reason for hiding this comment

mssola Jan 25, 2018

Choose a reason for hiding this comment

mssola Jan 25, 2018

Choose a reason for hiding this comment

mssola Jan 25, 2018

Choose a reason for hiding this comment

mssola Jan 25, 2018

Choose a reason for hiding this comment

mssola left a comment

Choose a reason for hiding this comment

mssola Jan 29, 2018

Choose a reason for hiding this comment

mssola commented Feb 12, 2018