Merge branch '61156-instance-level-cluster-pod-terminal-access' into 'master'

Instance-level cluster pod terminal access

Closes #55489, #55488, #55487, and #61156

See merge request gitlab-org/gitlab-ce!28613

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
+      def calculate_reactive_cache_for(environment)
+        return unless enabled?
 
-          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
+        { pods: read_pods(environment.deployment_namespace) }
       end
 
-      # Caches resources in the namespace so other calls don't need to block on
-      # network access
-      def calculate_reactive_cache
-        return unless enabled?
-
-        # We may want to cache extra things in the future
-        { pods: read_pods }
+      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 }
 

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?

changelogs/unreleased/61156-instance-level-cluster-pod-terminal-access.yml

+---
+title: Enable terminals for instance and group clusters
+merge_request: 28613
+author:
+type: added

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

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

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,18 +577,28 @@ describe Environment do
   describe '#terminals' do
     subject { environment.terminals }
 
-    context 'when the environment has terminals' do
     before do
-        allow(environment).to receive(:has_terminals?).and_return(true)
+      allow(environment).to receive(:deployment_platform).and_return(double)
     end
 
-      context 'when user configured kubernetes from CI/CD > Clusters' do
-        let!(:cluster) { create(:cluster, :project, :provided_by_gcp) }
-        let(:project) { cluster.project }
+    context 'reactive cache is empty' do
+      before do
+        stub_reactive_cache(environment, nil)
+      end
+
+      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
+      it 'retrieves terminals from the deployment platform' do
         expect(environment.deployment_platform)
-            .to receive(:terminals).with(environment)
+          .to receive(:terminals).with(environment, cache_data)
           .and_return(:fake_terminals)
 
         is_expected.to eq(:fake_terminals)
@@ -592,13 +606,36 @@ describe Environment do
     end
   end
 
-    context 'when the environment does not have terminals' do
+  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 '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

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