From 79e8caea450f7d4727fbc8f75ce1970bff233895 Mon Sep 17 00:00:00 2001
From: Sebastian Serth <Sebastian.Serth@hpi.de>
Date: Thu, 4 Nov 2021 10:40:55 +0100
Subject: [PATCH] Remove outdated execution environments after syncing all

---
 .../execution_environments_controller.rb      | 29 +++++++++++++++++--
 .../execution_environments_controller_spec.rb | 13 +++++++--
 2 files changed, 38 insertions(+), 4 deletions(-)

diff --git a/app/controllers/execution_environments_controller.rb b/app/controllers/execution_environments_controller.rb
index 4dc00502..5bc8328e 100644
--- a/app/controllers/execution_environments_controller.rb
+++ b/app/controllers/execution_environments_controller.rb
@@ -171,11 +171,36 @@ class ExecutionEnvironmentsController < ApplicationController
 
     return unless Runner.management_active?
 
-    success = ExecutionEnvironment.all.map do |execution_environment|
+    success = []
+
+    begin
+      # Get a list of all existing execution environments and mark them as a potential candidate for removal
+      environments_to_remove = Runner.strategy_class.environments.pluck(:id)
+      success << true
+    rescue Runner::Error => e
+      Rails.logger.debug { "Runner error while getting all execution environments: #{e.message}" }
+      environments_to_remove = []
+      success << false
+    end
+
+    success += ExecutionEnvironment.all.map do |execution_environment|
+      # Sync all current execution environments and prevent deletion of those just synced
+      environments_to_remove -= [execution_environment.id]
       Runner.strategy_class.sync_environment(execution_environment)
-    rescue Runner::Error
+    rescue Runner::Error => e
+      Rails.logger.debug { "Runner error while synchronizing execution environment with id #{execution_environment.id}: #{e.message}" }
       false
     end
+
+    success += environments_to_remove.map do |execution_environment_id|
+      # Remove execution environments not synced. We temporarily use a record which is not persisted
+      execution_environment = ExecutionEnvironment.new(id: execution_environment_id)
+      Runner.strategy_class.remove_environment(execution_environment)
+    rescue Runner::Error => e
+      Rails.logger.debug { "Runner error while deleting execution environment with id #{execution_environment.id}: #{e.message}" }
+      false
+    end
+
     if success.all?
       redirect_to ExecutionEnvironment, notice: t('execution_environments.index.synchronize_all.success')
     else
diff --git a/spec/controllers/execution_environments_controller_spec.rb b/spec/controllers/execution_environments_controller_spec.rb
index 9aee3e5a..001f1fa4 100644
--- a/spec/controllers/execution_environments_controller_spec.rb
+++ b/spec/controllers/execution_environments_controller_spec.rb
@@ -216,7 +216,8 @@ describe ExecutionEnvironmentsController do
   end
 
   describe '#sync_all_to_runner_management' do
-    let(:execution_environments) { FactoryBot.build_list(:ruby, 3) }
+    let(:execution_environments) { %i[ruby java python].map {|environment| FactoryBot.create(environment) } }
+    let(:outdated_execution_environments) { %i[node_js html].map {|environment| FactoryBot.build_stubbed(environment) } }
 
     let(:codeocean_config) { instance_double(CodeOcean::Config) }
     let(:runner_management_config) { {runner_management: {enabled: true, strategy: :poseidon}} }
@@ -229,11 +230,19 @@ describe ExecutionEnvironmentsController do
     end
 
     it 'copies all execution environments to the runner management' do
-      allow(ExecutionEnvironment).to receive(:all).and_return(execution_environments)
+      allow(Runner::Strategy::Poseidon).to receive(:environments).and_return(outdated_execution_environments)
+      expect(Runner::Strategy::Poseidon).to receive(:environments).once
 
       execution_environments.each do |execution_environment|
         allow(Runner::Strategy::Poseidon).to receive(:sync_environment).with(execution_environment).and_return(true)
         expect(Runner::Strategy::Poseidon).to receive(:sync_environment).with(execution_environment).once
+        expect(Runner::Strategy::Poseidon).not_to receive(:remove_environment).with(execution_environment)
+      end
+
+      outdated_execution_environments.each do |execution_environment|
+        allow(Runner::Strategy::Poseidon).to receive(:remove_environment).with(execution_environment).and_return(true)
+        expect(Runner::Strategy::Poseidon).to receive(:remove_environment).with(execution_environment).once
+        expect(Runner::Strategy::Poseidon).not_to receive(:sync_environment).with(execution_environment)
       end
 
       post :sync_all_to_runner_management