Fix out of sync with KubernetesService. Remove namespace pramas from controller. Use time_with_zone in schema. Remove Gcp::Clusters from safe_model_attributes.ym

7 files changed, 34 insertions, 71 deletions

diff --git a/app/controllers/projects/clusters_controller.rb b/app/controllers/projects/clusters_controller.rb
index 7570da3e0b1..c0fe8249d8b 100644
--- a/app/controllers/projects/clusters_controller.rb
+++ b/app/controllers/projects/clusters_controller.rb
@@ -94,9 +94,6 @@ class Projects::ClustersController < Projects::ApplicationController
       :name,
       :platform_type,
       :provider_type,
-      platform_kubernetes_attributes: [
-        :namespace
-      ],
       provider_gcp_attributes: [
         :gcp_project_id,
         :zone,
@@ -106,11 +103,7 @@ class Projects::ClustersController < Projects::ApplicationController
   end
 
   def update_params
-    params.require(:cluster).permit(
-      :enabled,
-      platform_kubernetes_attributes: [
-        :namespace
-      ])
+    params.require(:cluster).permit(:enabled)
   end
 
   def authorize_google_api
diff --git a/app/models/clusters/cluster.rb b/app/models/clusters/cluster.rb
index ca09b939f34..955dba51745 100644
--- a/app/models/clusters/cluster.rb
+++ b/app/models/clusters/cluster.rb
@@ -21,10 +21,16 @@ module Clusters
     validates :name, cluster_name: true
     validate :restrict_modification, on: :update
 
+    # TODO: Move back this into Clusters::Platforms::Kubernetes in 10.3
+    # We need callback here because `enabled` belongs to Clusters::Cluster
+    # Callbacks in Clusters::Platforms::Kubernetes will not be called after update
+    after_save :update_kubernetes_integration!
+
     delegate :status, to: :provider, allow_nil: true
     delegate :status_reason, to: :provider, allow_nil: true
     delegate :status_name, to: :provider, allow_nil: true
     delegate :on_creation?, to: :provider, allow_nil: true
+    delegate :update_kubernetes_integration!, to: :platform, allow_nil: true
 
     enum platform_type: {
       kubernetes: 1
diff --git a/app/models/clusters/platforms/kubernetes.rb b/app/models/clusters/platforms/kubernetes.rb
index 3ad2ffe531d..8b00f1dac0b 100644
--- a/app/models/clusters/platforms/kubernetes.rb
+++ b/app/models/clusters/platforms/kubernetes.rb
@@ -1,8 +1,6 @@
 module Clusters
   module Platforms
     class Kubernetes < ActiveRecord::Base
-      include Gitlab::CurrentSettings
-
       self.table_name = 'cluster_platforms_kubernetes'
 
       belongs_to :cluster, inverse_of: :platform_kubernetes, class_name: 'Clusters::Cluster'
@@ -28,13 +26,10 @@ module Clusters
         }
 
       # We expect to be `active?` only when enabled and cluster is created (the api_url is assigned)
-      with_options presence: true, if: :enabled? do
-        validates :api_url, url: true, presence: true
-        validates :token, presence: true
-      end
+      validates :api_url, url: true, presence: true
+      validates :token, presence: true
 
       # TODO: Glue code till we migrate Kubernetes Integration into Platforms::Kubernetes
-      after_save :update_kubernetes_integration!
       after_destroy :destroy_kubernetes_integration!
 
       alias_attribute :ca_pem, :ca_cert
@@ -60,6 +55,18 @@ module Clusters
         self.class.namespace_for_project(project) if project
       end
 
+      def update_kubernetes_integration!
+        raise 'Kubernetes service already configured' unless manages_kubernetes_service?
+
+        ensure_kubernetes_service.update!(
+          active: enabled?,
+          api_url: api_url,
+          namespace: namespace,
+          token: token,
+          ca_pem: ca_cert
+        )
+      end
+
       private
 
       def enforce_namespace_to_lower_case
@@ -79,24 +86,12 @@ module Clusters
         kubernetes_service.destroy!
       end
 
-      def update_kubernetes_integration!
-        return raise 'Kubernetes service already configured' unless manages_kubernetes_service?
-
-        ensure_kubernetes_service.update!(
-          active: enabled?,
-          api_url: api_url,
-          namespace: namespace,
-          token: token,
-          ca_pem: ca_cert,
-        )
-      end
-
       def kubernetes_service
-        @kubernetes_service ||= project.kubernetes_service || project.build_kubernetes_service
+        @kubernetes_service ||= project&.kubernetes_service
       end
 
       def ensure_kubernetes_service
-        @kubernetes_service ||= kubernetes_service || project.build_kubernetes_service
+        @kubernetes_service ||= kubernetes_service || project&.build_kubernetes_service
       end
     end
   end
diff --git a/app/services/clusters/create_service.rb b/app/services/clusters/create_service.rb
index a1c74566d7a..1d407739b21 100644
--- a/app/services/clusters/create_service.rb
+++ b/app/services/clusters/create_service.rb
@@ -13,11 +13,7 @@ module Clusters
     private
 
     def create_cluster
-      Clusters::Cluster.create!(
-        cluster_params.merge(
-          projects: [project]))
-    rescue ActiveRecord::RecordInvalid => e
-      e.record
+      Clusters::Cluster.create(cluster_params)
     end
 
     def cluster_params
@@ -27,7 +23,7 @@ module Clusters
         provider[:access_token] = access_token
       end
 
-      @cluster_params = params.merge(user: current_user)
+      @cluster_params = params.merge(user: current_user, projects: [project])
     end
   end
 end
diff --git a/app/services/clusters/gcp/finalize_creation_service.rb b/app/services/clusters/gcp/finalize_creation_service.rb
index 53b13518771..cea56f4e849 100644
--- a/app/services/clusters/gcp/finalize_creation_service.rb
+++ b/app/services/clusters/gcp/finalize_creation_service.rb
@@ -9,11 +9,9 @@ module Clusters
         configure_provider
         configure_kubernetes
 
-        provider.make_created!
+        cluster.save!
       rescue Google::Apis::ServerError, Google::Apis::ClientError, Google::Apis::AuthorizationError => e
         provider.make_errored!("Failed to request to CloudPlatform; #{e.message}")
-      rescue KubeException => e
-        provider.make_errored!("Failed to request to Kubernetes; #{e.message}")
       rescue ActiveRecord::RecordInvalid => e
         provider.make_errored!("Failed to configure GKE Cluster: #{e.message}")
       end
@@ -22,6 +20,7 @@ module Clusters
 
       def configure_provider
         provider.endpoint = gke_cluster.endpoint
+        provider.status_event = :make_created
       end
 
       def configure_kubernetes
@@ -39,7 +38,7 @@ module Clusters
           'https://' + gke_cluster.endpoint,
           Base64.decode64(gke_cluster.master_auth.cluster_ca_certificate),
           gke_cluster.master_auth.username,
-          gke_cluster.master_auth.password)
+          gke_cluster.master_auth.password).execute
       end
 
       def gke_cluster
diff --git a/db/schema.rb b/db/schema.rb
index d76977d45f2..914740ffa0e 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -464,8 +464,8 @@ ActiveRecord::Schema.define(version: 20171017145932) do
 
   create_table "cluster_platforms_kubernetes", force: :cascade do |t|
     t.integer "cluster_id", null: false
-    t.datetime "created_at", null: false
-    t.datetime "updated_at", null: false
+    t.datetime_with_timezone "created_at", null: false
+    t.datetime_with_timezone "updated_at", null: false
     t.text "api_url"
     t.text "ca_cert"
     t.string "namespace"
@@ -492,8 +492,8 @@ ActiveRecord::Schema.define(version: 20171017145932) do
     t.integer "cluster_id", null: false
     t.integer "status"
     t.integer "num_nodes", null: false
-    t.datetime "created_at", null: false
-    t.datetime "updated_at", null: false
+    t.datetime_with_timezone "created_at", null: false
+    t.datetime_with_timezone "updated_at", null: false
     t.text "status_reason"
     t.string "gcp_project_id", null: false
     t.string "zone", null: false
@@ -510,8 +510,8 @@ ActiveRecord::Schema.define(version: 20171017145932) do
     t.integer "user_id", null: false
     t.integer "provider_type"
     t.integer "platform_type"
-    t.datetime "created_at", null: false
-    t.datetime "updated_at", null: false
+    t.datetime_with_timezone "created_at", null: false
+    t.datetime_with_timezone "updated_at", null: false
     t.boolean "enabled", default: true
     t.string "name", null: false
   end
diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml
index d8dc3672d40..4b79e9f18c6 100644
--- a/spec/lib/gitlab/import_export/safe_model_attributes.yml
+++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml
@@ -356,32 +356,6 @@ Clusters::Platforms::Kubernetes:
 - encrypted_token_iv
 - created_at
 - updated_at
-Gcp::Cluster:
-- id
-- project_id
-- user_id
-- service_id
-- enabled
-- status
-- status_reason
-- project_namespace
-- endpoint
-- ca_cert
-- encrypted_kubernetes_token
-- encrypted_kubernetes_token_iv
-- username
-- encrypted_password
-- encrypted_password_iv
-- gcp_project_id
-- gcp_cluster_zone
-- gcp_cluster_name
-- gcp_cluster_size
-- gcp_machine_type
-- gcp_operation_id
-- encrypted_gcp_token
-- encrypted_gcp_token_iv
-- created_at
-- updated_at
 DeployKey:
 - id
 - user_id