From b129f06733c7994fb81cef4d0bae6d6611647a83 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Thu, 2 Nov 2017 23:19:11 +0900 Subject: [PATCH] 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 --- .../projects/clusters_controller.rb | 9 +---- app/models/clusters/cluster.rb | 6 +++ app/models/clusters/platforms/kubernetes.rb | 37 ++++++++----------- app/services/clusters/create_service.rb | 8 +--- .../clusters/gcp/finalize_creation_service.rb | 7 ++-- db/schema.rb | 12 +++--- .../import_export/safe_model_attributes.yml | 26 ------------- 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 -- GitLab