Project

General

Profile

Actions

Bug #19472

closed

installer needs to set up log rotation for /var/www/arvados-api/shared/log/*.log

Added by Peter Amstutz over 1 year ago. Updated over 1 year ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Category:
Deployment
Target version:
Story points:
-
Release relationship:
Auto

Subtasks 2 (1 open1 closed)

Task #19500: Review 19472-logrotateResolvedPeter Amstutz09/09/2022Actions
Task #19511: ReviewNewPeter Amstutz09/09/2022Actions
Actions #1

Updated by Peter Amstutz over 1 year ago

  • Status changed from New to In Progress
Actions #2

Updated by Lucas Di Pentima over 1 year ago

  • Assigned To set to Lucas Di Pentima
Actions #3

Updated by Lucas Di Pentima over 1 year ago

Updates at 720f774 - branch 19472-logrotate

This branch was created from 2.4-release, because main's provision script has somehow diverged and it's failing. I'll fix it in a followup branch.

  • Adds logrotate configs for api & workbench.
  • Pins salt version to v3004 (for now, we should check if we can make everything work with the latest stable).
  • Avoids re-downloading salt formulas.
  • Removes a missing package dependency workaround that is no longer needed because #17352 is already resolved.

This branch should be merged directly into 2.4-release

Actions #4

Updated by Peter Amstutz over 1 year ago

Lucas Di Pentima wrote in #note-3:

Updates at 720f774 - branch 19472-logrotate

This branch was created from 2.4-release, because main's provision script has somehow diverged and it's failing. I'll fix it in a followup branch.

  • Adds logrotate configs for api & workbench.

Could you add a link to where the "config" options are documented in case someone wants to edit them? I'm just guessing here but it looks like it keeps one year of logs, rotated daily, with old logs compressed.

  • Pins salt version to v3004 (for now, we should check if we can make everything work with the latest stable).

That makes sense.

  • Avoids re-downloading salt formulas.

I would add a "git fetch" before "git checkout", if we change the version of the formula, we might not have it. Or we could be a little more clever and only do it when the version tag isn't in the local repo.

  • Removes a missing package dependency workaround that is no longer needed because #17352 is already resolved.

Great!

This branch should be merged directly into 2.4-release

Actions #5

Updated by Lucas Di Pentima over 1 year ago

Updates at ae74e85

  • Adds a conditional git fetch if the formula repository was already cloned, to allow the subsequent git checkout work with newer tags.
  • Adds logrotate-formula's README reference to its pillar files to help a site admin who wants to tweak its configurations.
Actions #6

Updated by Peter Amstutz over 1 year ago

Lucas Di Pentima wrote in #note-5:

Updates at ae74e85

  • Adds a conditional git fetch if the formula repository was already cloned, to allow the subsequent git checkout work with newer tags.
  • Adds logrotate-formula's README reference to its pillar files to help a site admin who wants to tweak its configurations.

This LGTM, thanks!

Actions #7

Updated by Peter Amstutz over 1 year ago

  • Status changed from In Progress to Resolved
Actions #8

Updated by Peter Amstutz over 1 year ago

  • Status changed from Resolved to In Progress
Actions #9

Updated by Lucas Di Pentima over 1 year ago

I've been trying to understand why there's a divergence in the tools/salt-install/ dir between main & 2.4-release branches. It seems that throughout different months, changes have been applied to them separately.
I tend to think that the 2.4-release version is the version in use, but changes in the main version don't seem to be discardable.

Here's what I get when I run git diff main..1e0108e1a tools/salt-install/

diff --git a/tools/salt-install/config_examples/multi_host/aws/pillars/arvados.sls b/tools/salt-install/config_examples/multi_host/aws/pillars/arvados.sls
index 02653082f..941bd95f1 100644
--- a/tools/salt-install/config_examples/multi_host/aws/pillars/arvados.sls
+++ b/tools/salt-install/config_examples/multi_host/aws/pillars/arvados.sls
@@ -1,5 +1,3 @@
-# -*- coding: utf-8 -*-
-# vim: ft=yaml
 ---
 # Copyright (C) The Arvados Authors. All rights reserved.
 #
@@ -74,20 +72,13 @@ arvados:
       host: __DATABASE_INT_IP__
       password: "__DATABASE_PASSWORD__" 
       user: __CLUSTER___arvados
-      extra_conn_params:
-        client_encoding: UTF8
-      # Centos7 does not enable SSL by default, so we disable
-      # it here just for testing of the formula purposes only.
-      # You should not do this in production, and should
-      # configure Postgres certificates correctly
-      {%- if grains.os_family in ('RedHat',) %}
-        sslmode: disable
-      {%- endif %}
+      encoding: en_US.utf8
+      client_encoding: UTF8

     tls:
       # certificate: ''
       # key: ''
-      # When using arvados-snakeoil certs set insecure: true
+      # required to test with arvados-snakeoil certs
       insecure: false

     resources:
diff --git a/tools/salt-install/config_examples/multi_host/aws/pillars/arvados_development.sls b/tools/salt-install/config_examples/multi_host/aws/pillars/arvados_development.sls
deleted file mode 100644
index 6163cd360..000000000
--- a/tools/salt-install/config_examples/multi_host/aws/pillars/arvados_development.sls
+++ /dev/null
@@ -1,182 +0,0 @@
-# -*- coding: utf-8 -*-
-# vim: ft=yaml
----
-# Copyright (C) The Arvados Authors. All rights reserved.
-#
-# SPDX-License-Identifier: AGPL-3.0
-
-# This config file is used to test a multi-node deployment using a local
-# dispatcher. This setup is not recommended for production use.
-
-# The variables commented out are the default values that the formula uses.
-# The uncommented values are REQUIRED values. If you don't set them, running
-# this formula will fail.
-arvados:
-  ### GENERAL CONFIG
-  version: '__VERSION__'
-  ## It makes little sense to disable this flag, but you can, if you want :)
-  # use_upstream_repo: true
-
-  ## Repo URL is built with grains values. If desired, it can be completely
-  ## overwritten with the pillar parameter 'repo_url'
-  # repo:
-  #   humanname: Arvados Official Repository
-
-  release: __RELEASE__
-
-  ## IMPORTANT!!!!!
-  ## api, workbench and shell require some gems, so you need to make sure ruby
-  ## and deps are installed in order to install and compile the gems.
-  ## We default to `false` in these two variables as it's expected you already
-  ## manage OS packages with some other tool and you don't want us messing up
-  ## with your setup.
-  ruby:
-    ## We set these to `true` here for testing purposes.
-    ## They both default to `false`.
-    manage_ruby: true
-    manage_gems_deps: true
-    # pkg: ruby
-    # gems_deps:
-    #     - curl
-    #     - g++
-    #     - gcc
-    #     - git
-    #     - libcurl4
-    #     - libcurl4-gnutls-dev
-    #     - libpq-dev
-    #     - libxml2
-    #     - libxml2-dev
-    #     - make
-    #     - python3-dev
-    #     - ruby-dev
-    #     - zlib1g-dev
-
-  # config:
-  #   file: /etc/arvados/config.yml
-  #   user: root
-  ## IMPORTANT!!!!!
-  ## If you're intalling any of the rails apps (api, workbench), the group
-  ## should be set to that of the web server, usually `www-data`
-  #   group: root
-  #   mode: 640
-
-  ### ARVADOS CLUSTER CONFIG
-  cluster:
-    name: __CLUSTER__
-    domain: __DOMAIN__
-
-    database:
-      # max concurrent connections per arvados server daemon
-      # connection_pool_max: 32
-      name: __CLUSTER___arvados
-      host: 127.0.0.1
-      password: "__DATABASE_PASSWORD__" 
-      user: __CLUSTER___arvados
-      extra_conn_params:
-        client_encoding: UTF8
-      # Centos7 does not enable SSL by default, so we disable
-      # it here just for testing of the formula purposes only.
-      # You should not do this in production, and should
-      # configure Postgres certificates correctly
-      {%- if grains.os_family in ('RedHat',) %}
-        sslmode: disable
-      {%- endif %}
-
-    tls:
-      # certificate: ''
-      # key: ''
-      # When using arvados-snakeoil certs set insecure: true
-      insecure: true
-
-    resources:
-      virtual_machines:
-        shell:
-          name: shell
-          backend: __SHELL_INT_IP__
-          port: 4200
-
-    ### TOKENS
-    tokens:
-      system_root: __SYSTEM_ROOT_TOKEN__
-      management: __MANAGEMENT_TOKEN__
-      anonymous_user: __ANONYMOUS_USER_TOKEN__
-
-    ### KEYS
-    secrets:
-      blob_signing_key: __BLOB_SIGNING_KEY__
-      workbench_secret_key: __WORKBENCH_SECRET_KEY__
-
-    Login:
-      Test:
-        Enable: true
-        Users:
-          __INITIAL_USER__:
-            Email: __INITIAL_USER_EMAIL__
-            Password: __INITIAL_USER_PASSWORD__
-
-    ### VOLUMES
-    ## This should usually match all your `keepstore` instances
-    Volumes:
-      # the volume name will be composed with
-      # <cluster>-nyw5e-<volume>
-      __CLUSTER__-nyw5e-000000000000000:
-        AccessViaHosts:
-          'http://__KEEPSTORE0_INT_IP__:25107':
-            ReadOnly: false
-        Replication: 2
-        Driver: Directory
-        DriverParameters:
-          Root: /tmp
-      __CLUSTER__-nyw5e-000000000000001:
-        AccessViaHosts:
-          'http://__KEEPSTORE1_INT_IP__:25107':
-            ReadOnly: false
-        Replication: 2
-        Driver: Directory
-        DriverParameters:
-          Root: /tmp
-
-    Containers:
-      LocalKeepBlobBuffersPerVCPU: 0
-
-    Users:
-      NewUsersAreActive: true
-      AutoAdminFirstUser: true
-      AutoSetupNewUsers: true
-      AutoSetupNewUsersWithRepository: true
-
-    Services:
-      Controller:
-        ExternalURL: 'https://__CLUSTER__.__DOMAIN__:__CONTROLLER_EXT_SSL_PORT__'
-        InternalURLs:
-          'http://localhost:8003': {}
-      Keepbalance:
-        InternalURLs:
-          'http://__CONTROLLER_INT_IP__:9005': {}
-      Keepproxy:
-        ExternalURL: 'https://__CLUSTER__.__DOMAIN__:__KEEP_EXT_SSL_PORT__'
-        InternalURLs:
-          'http://__KEEP_INT_IP__:25100': {}
-      Keepstore:
-        InternalURLs:
-          'http://__KEEPSTORE0_INT_IP__:25107': {}
-          'http://__KEEPSTORE1_INT_IP__:25107': {}
-      RailsAPI:
-        InternalURLs:
-          'http://localhost:8004': {}
-      WebDAV:
-        ExternalURL: 'https://__CLUSTER__.__DOMAIN__:__KEEPWEB_EXT_SSL_PORT__'
-        InternalURLs:
-          'http://localhost:9002': {}
-      WebDAVDownload:
-        ExternalURL: 'https://__CLUSTER__.__DOMAIN__:__KEEPWEB_EXT_SSL_PORT__'
-      WebShell:
-        ExternalURL: 'https://__CLUSTER__.__DOMAIN__:__WEBSHELL_EXT_SSL_PORT__'
-      Websocket:
-        ExternalURL: 'wss://__CLUSTER__.__DOMAIN__:__WEBSOCKET_EXT_SSL_PORT__/websocket'
-        InternalURLs:
-          'http://__WEBSOCKET_INT_IP__:8005': {}
-      Workbench1:
-        ExternalURL: 'https://__CLUSTER__.__DOMAIN__:__WORKBENCH1_EXT_SSL_PORT__'
-      Workbench2:
-        ExternalURL: 'https://__CLUSTER__.__DOMAIN__:__WORKBENCH2_EXT_SSL_PORT__'
diff --git a/tools/salt-install/provision.sh b/tools/salt-install/provision.sh
index 638e5de80..93d4fcfd3 100755
--- a/tools/salt-install/provision.sh
+++ b/tools/salt-install/provision.sh
@@ -12,17 +12,6 @@

 set -o pipefail

-_exit_handler() {
-  local rc="$?" 
-  trap - EXIT
-  if [ "$rc" -ne 0 ]; then
-    echo "Error occurred ($rc) while running $0 at line $1 : $BASH_COMMAND" 
-  fi
-  exit "$rc" 
-}
-
-trap '_exit_handler $LINENO' EXIT ERR
-
 # capture the directory that the script is running from
 SCRIPT_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )" 

@@ -208,8 +197,8 @@ CUSTOM_CERTS_DIR="${SCRIPT_DIR}/local_config_dir/certs" 
 # release.
 # The "local.params.example.*" files already set "RELEASE=production" 
 # to deploy  production-ready packages
-RELEASE="development" 
-VERSION="latest" 
+RELEASE="production" 
+VERSION="2.4.2-1" 

 # These are arvados-formula-related parameters
 # An arvados-formula tag. For a stable release, this should be a
@@ -366,9 +355,9 @@ echo "...arvados" 
 git clone --quiet https://git.arvados.org/arvados-formula.git ${F_DIR}/arvados

 # If we want to try a specific branch of the formula
-if [ "x${BRANCH}" != "x" -a $(git rev-parse --abbrev-ref HEAD) != "${BRANCH}" ]; then
+if [ "x${BRANCH}" != "x" ]; then
   ( cd ${F_DIR}/arvados && git checkout --quiet -t origin/"${BRANCH}" -b "${BRANCH}" )
-elif [ "x${ARVADOS_TAG}" != "x" -a $(git rev-parse --abbrev-ref HEAD) != "${ARVADOS_TAG}" ]; then
+elif [ "x${ARVADOS_TAG}" != "x" ]; then
 ( cd ${F_DIR}/arvados && git checkout --quiet tags/"${ARVADOS_TAG}" -b "${ARVADOS_TAG}" )
 fi

@@ -706,7 +695,6 @@ else
         sed -i "s/__NGINX_INSTALL_SOURCE__/${NGINX_INSTALL_SOURCE}/g" ${P_DIR}/nginx_passenger.sls
       ;;
       "controller" | "websocket" | "workbench" | "workbench2" | "webshell" | "keepweb" | "keepproxy")
-        NGINX_INSTALL_SOURCE="install_from_repo" 
         # States
         if [ "${R}" = "workbench" ]; then
           NGINX_INSTALL_SOURCE="install_from_phusionpassenger" 
@@ -787,7 +775,7 @@ else
                     s#__CERT_PEM__#/etc/nginx/ssl/arvados-${R}.pem#g;
                     s#__CERT_KEY__#/etc/nginx/ssl/arvados-${R}.key#g" \
             ${P_DIR}/nginx_${R}_configuration.sls
-            grep -q ${R}$ ${P_DIR}/extra_custom_certs.sls || echo "  - ${R}" >> ${P_DIR}/extra_custom_certs.sls
+            grep -q ${R} ${P_DIR}/extra_custom_certs.sls || echo "  - ${R}" >> ${P_DIR}/extra_custom_certs.sls
           fi
         fi
         # We need to tweak the Nginx's pillar depending whether we want plain nginx or nginx+passenger
@@ -832,17 +820,15 @@ fi

 # Leave a copy of the Arvados CA so the user can copy it where it's required
 if [ "$DEV_MODE" = "yes" ]; then
-  ARVADOS_SNAKEOIL_CA_DEST_FILE="${SCRIPT_DIR}/${CLUSTER}.${DOMAIN}-arvados-snakeoil-ca.pem" 
-
+  echo "Copying the Arvados CA certificate to the installer dir, so you can import it" 
   # If running in a vagrant VM, also add default user to docker group
   if [ "x${VAGRANT}" = "xyes" ]; then
+    cp /etc/ssl/certs/arvados-snakeoil-ca.pem /vagrant/${CLUSTER}.${DOMAIN}-arvados-snakeoil-ca.pem
+
     echo "Adding the vagrant user to the docker group" 
     usermod -a -G docker vagrant
-    ARVADOS_SNAKEOIL_CA_DEST_FILE="/vagrant/${CLUSTER}.${DOMAIN}-arvados-snakeoil-ca.pem" 
-  fi
-  if [ -f /etc/ssl/certs/arvados-snakeoil-ca.pem ]; then
-    echo "Copying the Arvados CA certificate to the installer dir, so you can import it" 
-    cp /etc/ssl/certs/arvados-snakeoil-ca.pem ${ARVADOS_SNAKEOIL_CA_DEST_FILE}
+  else
+    cp /etc/ssl/certs/arvados-snakeoil-ca.pem ${SCRIPT_DIR}/${CLUSTER}.${DOMAIN}-arvados-snakeoil-ca.pem
   fi
 fi

diff --git a/tools/salt-install/tests/run-test.sh b/tools/salt-install/tests/run-test.sh
index 42ab71664..cf43273a1 100755
--- a/tools/salt-install/tests/run-test.sh
+++ b/tools/salt-install/tests/run-test.sh
@@ -9,6 +9,14 @@ export ARVADOS_API_HOST_INSECURE=true

 set -o pipefail

+# First, validate that the CA is installed and that we can query it with no errors.
+if ! curl -s -o /dev/null https://${ARVADOS_API_HOST}/users/welcome?return_to=%2F; then
+  echo "The Arvados CA was not correctly installed. Although some components will work," 
+  echo "others won't. Please verify that the CA cert file was installed correctly and" 
+  echo "retry running these tests." 
+  exit 1
+fi
+
 # https://doc.arvados.org/v2.0/install/install-jobs-image.html
 echo "Creating Arvados Standard Docker Images project" 
 uuid_prefix=$(arv --format=uuid user current | cut -d- -f1)

I think the main version failure happens because there's a trap statement that 2.4-release doesn't have, and a command in the middle of the execution fails because of a non-existent template file for the recently added shell role.

Actions #10

Updated by Peter Amstutz over 1 year ago

Keeping this open, haven't be able to confirm that logs are actually being rotated daily as intended.

Actions #11

Updated by Lucas Di Pentima over 1 year ago

  • Target version changed from 2022-09-14 sprint to 2022-09-28 sprint
Actions #12

Updated by Peter Amstutz over 1 year ago

Checked again after a couple more days and confirmed that log rotation is working, and the older logs are compressed by 80%-90% which is a huge win. So this can be resolved once the other changes are merged to main.

Actions #13

Updated by Lucas Di Pentima over 1 year ago

Updates at cfd1df6 - branch 19472-installer-logrotate
Test run: developer-run-tests: #3291 (just in case)

  • Syncs tools/salt-installer's previous state between 2.4-release & main branches.
  • Applies (via cherry-picking) branch previously merged to 2.4-release.
Actions #14

Updated by Lucas Di Pentima over 1 year ago

  • % Done changed from 50 to 100
  • Status changed from In Progress to Resolved
Actions #15

Updated by Peter Amstutz over 1 year ago

  • Release set to 53
Actions

Also available in: Atom PDF