From 90c27ea52ada6bc3f3a18bcf61f9c034c8cb65ba Mon Sep 17 00:00:00 2001
From: Tiger <twatson@gitlab.com>
Date: Wed, 22 May 2019 13:55:15 -0500
Subject: Move terminal construction logic to Environment

This enables terminals for group and project level clusters.
Previously there was no way to determine which project (and
therefore kubernetes namespace) to connect to, moving this
logic onto Environment means the assoicated project can be
used to look up the correct namespace.
---
 app/models/clusters/platforms/kubernetes.rb        | 49 +++++------------
 app/models/environment.rb                          | 19 ++++++-
 ...-instance-level-cluster-pod-terminal-access.yml |  5 ++
 doc/user/group/clusters/index.md                   |  8 ---
 spec/models/clusters/platforms/kubernetes_spec.rb  | 43 ++++++---------
 spec/models/environment_spec.rb                    | 63 +++++++++++++++++-----
 spec/workers/reactive_caching_worker_spec.rb       |  7 ++-
 7 files changed, 105 insertions(+), 89 deletions(-)
 create mode 100644 changelogs/unreleased/61156-instance-level-cluster-pod-terminal-access.yml

diff --git a/app/models/clusters/platforms/kubernetes.rb b/app/models/clusters/platforms/kubernetes.rb
index 5afb193cf86..9296c28776b 100644
--- a/app/models/clusters/platforms/kubernetes.rb
+++ b/app/models/clusters/platforms/kubernetes.rb
@@ -4,7 +4,6 @@ module Clusters
   module Platforms
     class Kubernetes < ApplicationRecord
       include Gitlab::Kubernetes
-      include ReactiveCaching
       include EnumWithNil
       include AfterCommitQueue
 
@@ -46,8 +45,6 @@ module Clusters
 
       validate :prevent_modification, on: :update
 
-      after_save :clear_reactive_cache!
-
       alias_attribute :ca_pem, :ca_cert
 
       delegate :enabled?, to: :cluster, allow_nil: true
@@ -96,27 +93,16 @@ module Clusters
         end
       end
 
-      # Constructs a list of terminals from the reactive cache
-      #
-      # Returns nil if the cache is empty, in which case you should try again a
-      # short time later
-      def terminals(environment)
-        with_reactive_cache do |data|
-          project = environment.project
-
-          pods = filter_by_project_environment(data[:pods], project.full_path_slug, environment.slug)
-          terminals = pods.flat_map { |pod| terminals_for_pod(api_url, cluster.kubernetes_namespace_for(project), pod) }.compact
-          terminals.each { |terminal| add_terminal_auth(terminal, terminal_auth) }
-        end
-      end
-
-      # Caches resources in the namespace so other calls don't need to block on
-      # network access
-      def calculate_reactive_cache
+      def calculate_reactive_cache_for(environment)
         return unless enabled?
 
-        # We may want to cache extra things in the future
-        { pods: read_pods }
+        { pods: read_pods(environment.deployment_namespace) }
+      end
+
+      def terminals(environment, data)
+        pods = filter_by_project_environment(data[:pods], environment.project.full_path_slug, environment.slug)
+        terminals = pods.flat_map { |pod| terminals_for_pod(api_url, environment.deployment_namespace, pod) }.compact
+        terminals.each { |terminal| add_terminal_auth(terminal, terminal_auth) }
       end
 
       def kubeclient
@@ -133,6 +119,12 @@ module Clusters
           ca_pem: ca_pem)
       end
 
+      def read_pods(namespace)
+        kubeclient.get_pods(namespace: namespace).as_json
+      rescue Kubeclient::ResourceNotFoundError
+        []
+      end
+
       def build_kube_client!
         raise "Incomplete settings" unless api_url
 
@@ -148,19 +140,6 @@ module Clusters
         )
       end
 
-      # Returns a hash of all pods in the namespace
-      def read_pods
-        # TODO: The project lookup here should be moved (to environment?),
-        # which will enable reading pods from the correct namespace for group
-        # and instance clusters.
-        # This will be done in https://gitlab.com/gitlab-org/gitlab-ce/issues/61156
-        return [] unless cluster.project_type?
-
-        kubeclient.get_pods(namespace: cluster.kubernetes_namespace_for(cluster.first_project)).as_json
-      rescue Kubeclient::ResourceNotFoundError
-        []
-      end
-
       def kubeclient_ssl_options
         opts = { verify_ssl: OpenSSL::SSL::VERIFY_PEER }
 
diff --git a/app/models/environment.rb b/app/models/environment.rb
index 1f7e8815c8e..b8ee54c1696 100644
--- a/app/models/environment.rb
+++ b/app/models/environment.rb
@@ -2,6 +2,8 @@
 
 class Environment < ApplicationRecord
   include Gitlab::Utils::StrongMemoize
+  include ReactiveCaching
+
   # Used to generate random suffixes for the slug
   LETTERS = ('a'..'z').freeze
   NUMBERS = ('0'..'9').freeze
@@ -17,6 +19,7 @@ class Environment < ApplicationRecord
   before_validation :generate_slug, if: ->(env) { env.slug.blank? }
 
   before_save :set_environment_type
+  after_save :clear_reactive_cache!
 
   validates :name,
             presence: true,
@@ -159,7 +162,21 @@ class Environment < ApplicationRecord
   end
 
   def terminals
-    deployment_platform.terminals(self) if has_terminals?
+    with_reactive_cache do |data|
+      deployment_platform.terminals(self, data)
+    end
+  end
+
+  def calculate_reactive_cache
+    return unless has_terminals? && !project.pending_delete?
+
+    deployment_platform.calculate_reactive_cache_for(self)
+  end
+
+  def deployment_namespace
+    strong_memoize(:kubernetes_namespace) do
+      deployment_platform&.kubernetes_namespace_for(project)
+    end
   end
 
   def has_metrics?
diff --git a/changelogs/unreleased/61156-instance-level-cluster-pod-terminal-access.yml b/changelogs/unreleased/61156-instance-level-cluster-pod-terminal-access.yml
new file mode 100644
index 00000000000..0b8d301352b
--- /dev/null
+++ b/changelogs/unreleased/61156-instance-level-cluster-pod-terminal-access.yml
@@ -0,0 +1,5 @@
+---
+title: Enable terminals for instance and group clusters
+merge_request: 28613
+author:
+type: added
diff --git a/doc/user/group/clusters/index.md b/doc/user/group/clusters/index.md
index 26d764fa2cf..8d4ffd93f59 100644
--- a/doc/user/group/clusters/index.md
+++ b/doc/user/group/clusters/index.md
@@ -138,14 +138,6 @@ The result will then be:
 - The Staging cluster will be used for the `deploy to staging` job.
 - The Production cluster will be used for the `deploy to production` job.
 
-## Unavailable features
-
-The following features are not currently available for group-level clusters:
-
-1. Terminals (see [related issue](https://gitlab.com/gitlab-org/gitlab-ce/issues/55487)).
-1. Pod logs (see [related issue](https://gitlab.com/gitlab-org/gitlab-ce/issues/55488)).
-1. Deployment boards (see [related issue](https://gitlab.com/gitlab-org/gitlab-ce/issues/55489)).
-
 <!-- ## Troubleshooting
 
 Include any troubleshooting steps that you can foresee. If you know beforehand what issues
diff --git a/spec/models/clusters/platforms/kubernetes_spec.rb b/spec/models/clusters/platforms/kubernetes_spec.rb
index 05b3035e591..471769e4aab 100644
--- a/spec/models/clusters/platforms/kubernetes_spec.rb
+++ b/spec/models/clusters/platforms/kubernetes_spec.rb
@@ -2,13 +2,11 @@
 
 require 'spec_helper'
 
-describe Clusters::Platforms::Kubernetes, :use_clean_rails_memory_store_caching do
+describe Clusters::Platforms::Kubernetes do
   include KubernetesHelpers
-  include ReactiveCachingHelpers
 
   it { is_expected.to belong_to(:cluster) }
   it { is_expected.to be_kind_of(Gitlab::Kubernetes) }
-  it { is_expected.to be_kind_of(ReactiveCaching) }
   it { is_expected.to respond_to :ca_pem }
 
   it { is_expected.to validate_exclusion_of(:namespace).in_array(%w(gitlab-managed-apps)) }
@@ -397,17 +395,16 @@ describe Clusters::Platforms::Kubernetes, :use_clean_rails_memory_store_caching
   end
 
   describe '#terminals' do
-    subject { service.terminals(environment) }
+    subject { service.terminals(environment, pods: pods) }
 
     let!(:cluster) { create(:cluster, :project, platform_kubernetes: service) }
     let(:project) { cluster.project }
     let(:service) { create(:cluster_platform_kubernetes, :configured) }
     let(:environment) { build(:environment, project: project, name: "env", slug: "env-000000") }
+    let(:pods) { [{ "bad" => "pod" }] }
 
     context 'with invalid pods' do
       it 'returns no terminals' do
-        stub_reactive_cache(service, pods: [{ "bad" => "pod" }])
-
         is_expected.to be_empty
       end
     end
@@ -416,13 +413,7 @@ describe Clusters::Platforms::Kubernetes, :use_clean_rails_memory_store_caching
       let(:pod) { kube_pod(environment_slug: environment.slug, namespace: cluster.kubernetes_namespace_for(project), project_slug: project.full_path_slug) }
       let(:pod_with_no_terminal) { kube_pod(environment_slug: environment.slug, project_slug: project.full_path_slug, status: "Pending") }
       let(:terminals) { kube_terminals(service, pod) }
-
-      before do
-        stub_reactive_cache(
-          service,
-          pods: [pod, pod, pod_with_no_terminal, kube_pod(environment_slug: "should-be-filtered-out")]
-        )
-      end
+      let(:pods) { [pod, pod, pod_with_no_terminal, kube_pod(environment_slug: "should-be-filtered-out")] }
 
       it 'returns terminals' do
         is_expected.to eq(terminals + terminals)
@@ -437,16 +428,18 @@ describe Clusters::Platforms::Kubernetes, :use_clean_rails_memory_store_caching
     end
   end
 
-  describe '#calculate_reactive_cache' do
-    subject { service.calculate_reactive_cache }
-
-    let!(:cluster) { create(:cluster, :project, enabled: enabled, platform_kubernetes: service) }
+  describe '#calculate_reactive_cache_for' do
     let(:service) { create(:cluster_platform_kubernetes, :configured) }
-    let(:enabled) { true }
-    let(:namespace) { cluster.kubernetes_namespace_for(cluster.project) }
+    let(:pod) { kube_pod }
+    let(:namespace) { pod["metadata"]["namespace"] }
+    let(:environment) { instance_double(Environment, deployment_namespace: namespace) }
 
-    context 'when cluster is disabled' do
-      let(:enabled) { false }
+    subject { service.calculate_reactive_cache_for(environment) }
+
+    context 'when the kubernetes integration is disabled' do
+      before do
+        allow(service).to receive(:enabled?).and_return(false)
+      end
 
       it { is_expected.to be_nil }
     end
@@ -457,7 +450,7 @@ describe Clusters::Platforms::Kubernetes, :use_clean_rails_memory_store_caching
         stub_kubeclient_deployments(namespace)
       end
 
-      it { is_expected.to include(pods: [kube_pod]) }
+      it { is_expected.to include(pods: [pod]) }
     end
 
     context 'when kubernetes responds with 500s' do
@@ -477,11 +470,5 @@ describe Clusters::Platforms::Kubernetes, :use_clean_rails_memory_store_caching
 
       it { is_expected.to include(pods: []) }
     end
-
-    context 'when the cluster is not project level' do
-      let(:cluster) { create(:cluster, :group, platform_kubernetes: service) }
-
-      it { is_expected.to include(pods: []) }
-    end
   end
 end
diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb
index 379dda1f5c4..fe4d64818b4 100644
--- a/spec/models/environment_spec.rb
+++ b/spec/models/environment_spec.rb
@@ -2,10 +2,14 @@
 
 require 'spec_helper'
 
-describe Environment do
+describe Environment, :use_clean_rails_memory_store_caching do
+  include ReactiveCachingHelpers
+
   let(:project) { create(:project, :stubbed_repository) }
   subject(:environment) { create(:environment, project: project) }
 
+  it { is_expected.to be_kind_of(ReactiveCaching) }
+
   it { is_expected.to belong_to(:project).required }
   it { is_expected.to have_many(:deployments) }
 
@@ -573,32 +577,65 @@ describe Environment do
   describe '#terminals' do
     subject { environment.terminals }
 
-    context 'when the environment has terminals' do
+    before do
+      allow(environment).to receive(:deployment_platform).and_return(double)
+    end
+
+    context 'reactive cache is empty' do
       before do
-        allow(environment).to receive(:has_terminals?).and_return(true)
+        stub_reactive_cache(environment, nil)
       end
 
-      context 'when user configured kubernetes from CI/CD > Clusters' do
-        let!(:cluster) { create(:cluster, :project, :provided_by_gcp) }
-        let(:project) { cluster.project }
+      it { is_expected.to be_nil }
+    end
+
+    context 'reactive cache has pod data' do
+      let(:cache_data) { Hash(pods: %w(pod1 pod2)) }
+
+      before do
+        stub_reactive_cache(environment, cache_data)
+      end
 
-        it 'returns the terminals from the deployment service' do
-          expect(environment.deployment_platform)
-            .to receive(:terminals).with(environment)
-            .and_return(:fake_terminals)
+      it 'retrieves terminals from the deployment platform' do
+        expect(environment.deployment_platform)
+          .to receive(:terminals).with(environment, cache_data)
+          .and_return(:fake_terminals)
 
-          is_expected.to eq(:fake_terminals)
-        end
+        is_expected.to eq(:fake_terminals)
       end
     end
+  end
+
+  describe '#calculate_reactive_cache' do
+    let(:cluster) { create(:cluster, :project, :provided_by_user) }
+    let(:project) { cluster.project }
+    let(:environment) { create(:environment, project: project) }
+    let!(:deployment) { create(:deployment, :success, environment: environment) }
+
+    subject { environment.calculate_reactive_cache }
+
+    it 'returns cache data from the deployment platform' do
+      expect(environment.deployment_platform).to receive(:calculate_reactive_cache_for)
+        .with(environment).and_return(pods: %w(pod1 pod2))
+
+      is_expected.to eq(pods: %w(pod1 pod2))
+    end
 
-    context 'when the environment does not have terminals' do
+    context 'environment does not have terminals available' do
       before do
         allow(environment).to receive(:has_terminals?).and_return(false)
       end
 
       it { is_expected.to be_nil }
     end
+
+    context 'project is pending deletion' do
+      before do
+        allow(environment.project).to receive(:pending_delete?).and_return(true)
+      end
+
+      it { is_expected.to be_nil }
+    end
   end
 
   describe '#has_metrics?' do
diff --git a/spec/workers/reactive_caching_worker_spec.rb b/spec/workers/reactive_caching_worker_spec.rb
index b8ca6063ccd..ca0e76fc19a 100644
--- a/spec/workers/reactive_caching_worker_spec.rb
+++ b/spec/workers/reactive_caching_worker_spec.rb
@@ -3,17 +3,16 @@
 require 'spec_helper'
 
 describe ReactiveCachingWorker do
-  let(:service) { project.deployment_platform }
-
   describe '#perform' do
     context 'when user configured kubernetes from CI/CD > Clusters' do
       let!(:cluster) { create(:cluster, :project, :provided_by_gcp) }
       let(:project) { cluster.project }
+      let!(:environment) { create(:environment, project: project) }
 
       it 'calls #exclusively_update_reactive_cache!' do
-        expect_any_instance_of(Clusters::Platforms::Kubernetes).to receive(:exclusively_update_reactive_cache!)
+        expect_any_instance_of(Environment).to receive(:exclusively_update_reactive_cache!)
 
-        described_class.new.perform("Clusters::Platforms::Kubernetes", service.id)
+        described_class.new.perform("Environment", environment.id)
       end
     end
   end
-- 
cgit v1.2.1