squashme: resolve conflicts, bring up to date

git-https-proxy/main.go

@@ -21,14 +21,18 @@ import (
 
 func main() {
 	config := parseEnv()
-	// INFO: Make a channel that will receive the SIGTERM
+	// INFO: Make a channel that will receive the SIGTERM on shutdown
 	sigTerm := make(chan os.Signal, 1)
-	signal.Notify(sigTerm, syscall.SIGINT, syscall.SIGTERM)
+	signal.Notify(sigTerm, syscall.SIGTERM, syscall.SIGINT)
 	ctx := context.Background()
+	// INFO: Used to coordinate shutdown between git-proxy and the session when the user
+	// is not anonymous and there may be an autosave branch that needs to be created
 	shutdownFlags := shutdownFlagsStruct{
 		sigtermReceived: false,
 		shutdownAllowed: false,
 	}
+
+	// INFO: Setup servers
 	proxyHandler := getProxyHandler(config)
 	proxyServer := http.Server{
 		Addr:    fmt.Sprintf(":%s", config.ProxyPort),
@@ -39,34 +43,55 @@ func main() {
 		Addr:    fmt.Sprintf(":%s", config.HealthPort),
 		Handler: healthHandler,
 	}
+
+	// INFO: Run servers in the background
 	go func() {
-		// INFO: Run the health server in the "background"
 		log.Printf("Health server active on port %s\n", config.HealthPort)
 		log.Fatalln(healthServer.ListenAndServe())
 	}()
 	go func() {
-		// INFO: Run the proxy server in the "background"
 		log.Printf("Git proxy active on port %s\n", config.ProxyPort)
 		log.Printf("Repo Url: %v, anonymous session: %v\n", config.RepoUrl, config.AnonymousSession)
 		log.Fatalln(proxyServer.ListenAndServe())
 	}()
-	// INFO: Block until you receive sitTerm
+
+	// INFO: Block until you receive sigTerm to shutdown. All of this is necessary
+	// because the proxy has to shut down only after all the other containers do so in case 
+	// any other containers (i.e. session or sidecar) need git right before shutting down, 
+	// and this is the case exactly for creating autosave branches.
 	<- sigTerm
-	log.Printf(
-		"SIGTERM received. Waiting for /shutdown to be called or timing out in %v\n", 
-		config.SessionTerminationGracePeriod,
-	)
-	// INFO: After sigterm is received update flags and wait for shutdown flag to show up
-	sigTermTime := time.Now()
-	shutdownFlags.lock.Lock()
-	shutdownFlags.sigtermReceived = true
-	shutdownFlags.lock.Unlock()
-	for {
-		if shutdownFlags.shutdownAllowed || (time.Now().Sub(sigTermTime) > config.SessionTerminationGracePeriod) {
-			healthServer.Shutdown(ctx)
-			proxyServer.Shutdown(ctx)
+	if config.AnonymousSession {
+		log.Print("SIGTERM received. Shutting down servers.\n")
+		healthServer.Shutdown(ctx)
+		proxyServer.Shutdown(ctx)
+	} else {
+		log.Printf(
+			"SIGTERM received. Waiting for /shutdown to be called or timing out in %v\n", 
+			config.SessionTerminationGracePeriod,
+		)
+		sigTermTime := time.Now()
+		shutdownFlags.lock.Lock()
+		shutdownFlags.sigtermReceived = true
+		shutdownFlags.lock.Unlock()
+		for {
+			if shutdownFlags.shutdownAllowed || (time.Now().Sub(sigTermTime) > config.SessionTerminationGracePeriod) {
+				log.Printf(
+					"Shutting down servers. SIGTERM received: %v, Shutdown allowed: %v.\n",
+					shutdownFlags.sigtermReceived,
+					shutdownFlags.shutdownAllowed,
+				)
+				err := healthServer.Shutdown(ctx)
+				if err != nil {
+					log.Fatalln(err)
+				}
+				err = proxyServer.Shutdown(ctx)
+				if err != nil {
+					log.Fatalln(err)
+				}
+				break
+			}
+			time.Sleep(time.Second * 5)
 		}
-		time.Sleep(time.Second * 5)
 	}
 }
 
@@ -93,10 +118,10 @@ func parseEnv() *gitProxyConfig {
 	var repoUrl *url.URL
 	var err error
 	var SessionTerminationGracePeriod time.Duration
-	if proxyPort, ok = os.LookupEnv("MITM_PROXY_PORT"); !ok {
+	if proxyPort, ok = os.LookupEnv("GIT_PROXY_PORT"); !ok {
 		proxyPort = "8080"
 	}
-	if healthPort, ok = os.LookupEnv("HEALTH_PORT"); !ok {
+	if healthPort, ok = os.LookupEnv("GIT_PROXY_HEALTH_PORT"); !ok {
 		healthPort = "8081"
 	}
 	if anonymousSessionStr, ok = os.LookupEnv("ANONYMOUS_SESSION"); !ok {

git_services/Dockerfile.init

@@ -3,7 +3,7 @@ FROM python:3.9-slim
 LABEL maintainer="Swiss Data Science Center <info@datascience.ch>"
 
 RUN apt-get update && \
-    apt-get install -y --no-install-recommends git=1:2.30.2-1 git-lfs=2.13.2-1+b5 curl && \
+    apt-get install -y --no-install-recommends git=1:2.30.2-1 git-lfs=2.13.2-1+b5 curl tini && \
     apt-get purge -y --auto-remove && \
     rm -rf /var/lib/apt/lists/* && \
     useradd jovyan -u1000 -g100 --create-home
@@ -20,4 +20,5 @@ ENV PATH "/home/jovyan/.local/bin:$PATH"
 RUN curl -sSL https://install.python-poetry.org | python3 - && \
     poetry install --no-dev
 
-ENTRYPOINT ["poetry", "run", "python", "-m", "git_services.init.clone"]
+ENTRYPOINT ["tini", "-g", "--"]
+CMD ["poetry", "run", "python", "-m", "git_services.init.clone"]

git_services/Dockerfile.sidecar

@@ -3,7 +3,7 @@ FROM python:3.9-slim
 LABEL maintainer="Swiss Data Science Center <info@datascience.ch>"
 
 RUN apt-get update && \
-    apt-get install -y --no-install-recommends git=1:2.30.2-1 git-lfs=2.13.2-1+b5 curl && \
+    apt-get install -y --no-install-recommends git=1:2.30.2-1 git-lfs=2.13.2-1+b5 curl tini && \
     apt-get purge -y --auto-remove && \
     rm -rf /var/lib/apt/lists/* && \
     useradd jovyan -u1000 -g100 --create-home
@@ -25,4 +25,5 @@ ENV HOST="0.0.0.0"
 #       even if this is an invalid rpc request.
 HEALTHCHECK CMD curl http://$HOST:4000
 
-ENTRYPOINT ["poetry", "run", "python", "-m", "git_services.sidecar.rpc_server"]
+ENTRYPOINT ["tini", "-g", "--"]
+CMD ["poetry", "run", "python", "-m", "git_services.sidecar.rpc_server"]

git_services/git_services/sidecar/rpc_server.py

@@ -1,11 +1,15 @@
-from git_services.sidecar.config import config_from_env
 from jsonrpc import JSONRPCResponseManager, dispatcher
 import os
+import requests
 from werkzeug.wrappers import Request, Response
 from werkzeug.serving import run_simple
 from pathlib import Path
+from subprocess import PIPE, Popen
+import shlex
+
 from git_services.cli import GitCLI
 from git_services.cli.sentry import setup_sentry
+from git_services.sidecar.config import config_from_env
 
 
 @dispatcher.add_method
@@ -53,40 +57,61 @@ def status(path: str = ".", **kwargs):
 @dispatcher.add_method
 def autosave(**kwargs):
     """Create an autosave branch with uncommitted work."""
-    repo_path = os.environ.get("MOUNT_PATH")
-    status_result = status(path=repo_path)
-    should_commit = not status_result["clean"]
-    should_push = status_result["ahead"] > 0
-
-    if not should_commit and not should_push:
-        return
+    try:
+        git_proxy_health_port = os.getenv("GIT_PROXY_HEALTH_PORT", "8081")
+        repo_path = os.environ.get("MOUNT_PATH")
+        status_result = status(path=repo_path)
+        should_commit = not status_result["clean"]
+        should_push = status_result["ahead"] > 0
 
-    initial_commit = os.environ["CI_COMMIT_SHA"][0:7]
-    current_commit = status_result["commit"][0:7]
-    current_branch = status_result["branch"]
+        if not should_commit and not should_push:
+            requests.get(f"http://localhost:{git_proxy_health_port}/shutdown")
+            return
 
-    user = os.environ["RENKU_USERNAME"]
+        initial_commit = os.environ["CI_COMMIT_SHA"][0:7]
+        current_commit = status_result["commit"][0:7]
+        current_branch = status_result["branch"]
 
-    autosave_branch_name = (
-        f"renku/autosave/{user}/{current_branch}/{initial_commit}/{current_commit}"
-    )
+        user = os.environ["RENKU_USERNAME"]
 
-    cli = GitCLI(Path(repo_path))
-
-    cli.git_checkout(f"-b {autosave_branch_name}")
-
-    if should_commit:
-        cli.git_add("-A")
-        cli.git_commit(
-            "--no-verify "
-            f"-m 'Auto-saving for {user} on branch {current_branch} from commit {initial_commit}'"
+        autosave_branch_name = (
+            f"renku/autosave/{user}/{current_branch}/{initial_commit}/{current_commit}"
         )
 
-    cli.git_push(f"origin {autosave_branch_name}")
-
-    cli.git_reset(f"--soft {current_branch}")
-    cli.git_checkout(f"{current_branch}")
-    cli.git_branch(f"-D {autosave_branch_name}")
+        cli = GitCLI(Path(repo_path))
+
+        cli.git_checkout(f"-b {autosave_branch_name}")
+
+        if should_commit:
+            # INFO: Find large files that should be checked in git LFS
+            autosave_min_file_size = os.getenv('AUTOSAVE_MINIMUM_LFS_FILE_SIZE_BYTES', '1000000')
+            cmd_res = Popen(
+                shlex.split(f"find . -type f -size +{autosave_min_file_size}c"),
+                cwd=Path(repo_path),
+                stdout=PIPE,
+                stderr=PIPE,
+            )
+            stdout, _ = cmd_res.communicate()
+            lfs_files = stdout.decode("utf-8").split()
+            if len(lfs_files) > 0:
+                cli.git_lfs("track " + " ".join(lfs_files))
+            cli.git_add("-A")
+            cli.git_commit(
+                "--no-verify "
+                f"-m 'Auto-saving for {user} on branch "
+                f"{current_branch} from commit {initial_commit}'"
+            )
+
+        cli.git_push(f"origin {autosave_branch_name}")
+
+        cli.git_reset(f"--soft {current_branch}")
+        cli.git_checkout(f"{current_branch}")
+        cli.git_branch(f"-D {autosave_branch_name}")
+    finally:
+        # INFO: Inform the proxy it can shut down
+        # NOTE: Do not place return, break or continue here, otherwise
+        # the exception from try will be completely discarded.
+        requests.get(f"http://localhost:{git_proxy_health_port}/shutdown")
 
 
 @Request.application

helm-chart/renku-notebooks/templates/statefulset.yaml

@@ -154,6 +154,8 @@ spec:
               value: {{ .Values.cloudstorage.s3.enabled | quote }}
             - name: SESSION_TERMINATION_GRACE_PERIOD_SECONDS
               value: {{ .Values.sessionAutosave.terminationGracePeriodSeconds | quote }}
+            - name: AUTOSAVE_MINIMUM_LFS_FILE_SIZE_BYTES
+              value: {{ .Values.sessionAutosave.minimumLFSFileSizeBytes | quote }}
             - name: NOTEBOOKS_SERVICE_VERSION
               value: {{ .Values.image.tag | quote }}
           ports:

helm-chart/renku-notebooks/templates/test.yaml

@@ -86,6 +86,8 @@ spec:
           value: {{ $.Values.cloudstorage.s3.enabled | quote }}
         - name: SESSION_TERMINATION_GRACE_PERIOD_SECONDS
           value: {{ $.Values.sessionAutosave.terminationGracePeriodSeconds | quote }}
+        - name: AUTOSAVE_MINIMUM_LFS_FILE_SIZE_BYTES
+          value: {{ $.Values.sessionAutosave.minimumLFSFileSizeBytes | quote }}
         - name: NOTEBOOKS_SERVICE_VERSION
           value: {{ $.Values.image.tag | quote }}
       command:

helm-chart/renku-notebooks/values.yaml

@@ -135,7 +135,7 @@ sessionIngress:
 
 sessionAutosave:
   ## Any file higher than the minimum will be added to LFS
-  minimumLFSFileSize: 5M
+  minimumLFSFileSizeBytes: 1000000
   ## How long should k8s wait for an autosave branch to be created before
   ## fully and forcefully removing a user session
   terminationGracePeriodSeconds: 600

renku_notebooks/api/amalthea_patches/git_proxy.py

@@ -26,9 +26,14 @@ def main(server):
                                 "name": "REPOSITORY_URL",
                                 "value": server.gl_project.http_url_to_repo,
                             },
-                            {"name": "MITM_PROXY_PORT", "value": "8080"},
-                            {"name": "HEALTH_PORT", "value": "8081"},
-                            {"name": "SHUTDOWN_PORT", "value": "8082"},
+                            {
+                                "name": "GIT_PROXY_PORT",
+                                "value": current_app.config["GIT_PROXY_PORT"],
+                            },
+                            {
+                                "name": "GIT_PROXY_HEALTH_PORT",
+                                "value": current_app.config["GIT_PROXY_HEALTH_PORT"],
+                            },
                             {
                                 "name": "GITLAB_OAUTH_TOKEN",
                                 "value": server._user.git_token,
@@ -41,13 +46,25 @@ def main(server):
                                     else "true"
                                 ),
                             },
+                            {
+                                "name": "SESSION_TERMINATION_GRACE_PERIOD_SECONDS",
+                                "value": str(current_app.config[
+                                    "SESSION_TERMINATION_GRACE_PERIOD_SECONDS"
+                                ]),
+                            },
                         ],
                         "livenessProbe": {
-                            "httpGet": {"path": "/health", "port": 8081},
+                            "httpGet": {
+                                "path": "/health",
+                                "port": int(current_app.config["GIT_PROXY_HEALTH_PORT"]),
+                            },
                             "initialDelaySeconds": 3,
                         },
                         "readinessProbe": {
-                            "httpGet": {"path": "/health", "port": 8081},
+                            "httpGet": {
+                                "path": "/health",
+                                "port": int(current_app.config["GIT_PROXY_HEALTH_PORT"]),
+                            },
                             "initialDelaySeconds": 3,
                         },
                         "volumeMounts": etc_cert_volume_mount,

renku_notebooks/api/amalthea_patches/git_sidecar.py

@@ -54,6 +54,21 @@ def main(server):
                                 "name": "RENKU_USERNAME",
                                 "value": f"{server._user.username}",
                             },
+                            # NOTE: The git proxy health port is also used to signal that the proxy
+                            # can safely shut down after any autosave branches have been properly
+                            # created.
+                            {
+                                "name": "GIT_PROXY_HEALTH_PORT",
+                                "value": current_app.config["GIT_PROXY_HEALTH_PORT"],
+                            },
+                            {
+                                "name": "AUTOSAVE_MINIMUM_LFS_FILE_SIZE_BYTES",
+                                "value": str(
+                                    current_app.config[
+                                        "AUTOSAVE_MINIMUM_LFS_FILE_SIZE_BYTES"
+                                    ]
+                                ),
+                            },
                         ],
                         # NOTE: Autosave Branch creation
                         "lifecycle": {

renku_notebooks/config.py

@@ -119,17 +119,20 @@
 
 IMAGE_DEFAULT_WORKDIR = "/home/jovyan"
 
+# NOTE: K8s can convert large numbers to scientific notation and the to strings
+# as they are passed down to the container through environment variables.
+# Python considers scientific notation as float, so int("1e6") raises an error.
 CULLING_REGISTERED_IDLE_SESSIONS_THRESHOLD_SECONDS = int(
-    os.getenv("CULLING_REGISTERED_IDLE_SESSIONS_THRESHOLD_SECONDS", 86400)
+    float(os.getenv("CULLING_REGISTERED_IDLE_SESSIONS_THRESHOLD_SECONDS", 86400))
 )
 CULLING_ANONYMOUS_IDLE_SESSIONS_THRESHOLD_SECONDS = int(
-    os.getenv("CULLING_ANONYMOUS_IDLE_SESSIONS_THRESHOLD_SECONDS", 43200)
+    float(os.getenv("CULLING_ANONYMOUS_IDLE_SESSIONS_THRESHOLD_SECONDS", 43200))
 )
 CULLING_REGISTERED_MAX_AGE_THRESHOLD_SECONDS = int(
-    os.getenv("CULLING_REGISTERED_MAX_AGE_THRESHOLD_SECONDS", 0)
+    float(os.getenv("CULLING_REGISTERED_MAX_AGE_THRESHOLD_SECONDS", 0))
 )
 CULLING_ANONYMOUS_MAX_AGE_THRESHOLD_SECONDS = int(
-    os.getenv("CULLING_ANONYMOUS_MAX_AGE_THRESHOLD_SECONDS", 0)
+    float(os.getenv("CULLING_ANONYMOUS_MAX_AGE_THRESHOLD_SECONDS", 0))
 )
 
 SESSION_NODE_SELECTOR = safe_load(os.environ.get("SESSION_NODE_SELECTOR", "{}"))
@@ -139,6 +142,21 @@
 CURRENT_RESOURCE_SCHEMA_VERSION = "1"
 S3_MOUNTS_ENABLED = os.getenv("S3_MOUNTS_ENABLED", "false").lower() == "true"
 NOTEBOOKS_SERVICE_VERSION = os.getenv("NOTEBOOKS_SERVICE_VERSION", "0.0.0")
+
 SESSION_TERMINATION_GRACE_PERIOD_SECONDS = int(
-    os.getenv("SESSION_TERMINATION_GRACE_PERIOD_SECONDS", 600)
+    float(os.getenv("SESSION_TERMINATION_GRACE_PERIOD_SECONDS", 600))
+)
+
+GIT_PROXY_PORT = "8080"
+"""The port for the proxy that injects user credentials (if needed) in Git requests."""
+
+GIT_PROXY_HEALTH_PORT = "8081"
+"""The port for the health checks of the Git proxy, also used to signal that it is 
+safe for the Git proxy to shut down. This shutdown is necessary because the Git proxy
+otherwise shuts down before an autosave branch can be created."""
+
+AUTOSAVE_MINIMUM_LFS_FILE_SIZE_BYTES = int(
+    float(os.getenv("AUTOSAVE_MINIMUM_LFS_FILE_SIZE_BYTES", 1000000))
 )
+"""Used to determine which files should be checked in LFS when creating an autosave in a
+session."""