From 087af654bbae1e4a843029b33e1aab546f4d7d61 Mon Sep 17 00:00:00 2001 From: Mayra Cabrera Date: Thu, 31 Jan 2019 08:58:58 -0600 Subject: [PATCH] Addresses backend/db review comments - Fixes multiple typos on AutoDevops script - Add an alias to Clusters::Cluster#domain as base_domain, so it's more descriptive - Removes unnecessary memoization on qa specs - Changes migration to a post migration to deal better with traffic on big instances (like gitlab.com) --- app/controllers/clusters/clusters_controller.rb | 4 ++-- app/models/clusters/cluster.rb | 2 ++ app/models/project_auto_devops.rb | 4 +++- app/views/clusters/clusters/_form.html.haml | 8 ++++---- .../projects/settings/ci_cd/_autodevops_form.html.haml | 2 +- .../52363-ui-changes-to-cluster-and-ado-pages.yml | 2 +- ...0_migrate_auto_dev_ops_domain_to_cluster_domain.rb} | 0 db/schema.rb | 2 +- lib/gitlab/ci/templates/Auto-DevOps.gitlab-ci.yml | 6 +++--- qa/qa/resource/kubernetes_cluster.rb | 10 ++-------- spec/controllers/groups/clusters_controller_spec.rb | 2 +- spec/features/clusters/cluster_detail_page_spec.rb | 2 +- ...grate_auto_dev_ops_domain_to_cluster_domain_spec.rb | 2 +- 13 files changed, 22 insertions(+), 24 deletions(-) rename db/{migrate/20190129165720_migrate_auto_dev_ops_domain_to_cluster_domain.rb => post_migrate/20190204115450_migrate_auto_dev_ops_domain_to_cluster_domain.rb} (100%) diff --git a/app/controllers/clusters/clusters_controller.rb b/app/controllers/clusters/clusters_controller.rb index 483842fe456..3bd91b71d92 100644 --- a/app/controllers/clusters/clusters_controller.rb +++ b/app/controllers/clusters/clusters_controller.rb @@ -127,7 +127,7 @@ class Clusters::ClustersController < Clusters::BaseController params.require(:cluster).permit( :enabled, :environment_scope, - :domain, + :base_domain, platform_kubernetes_attributes: [ :namespace ] @@ -137,7 +137,7 @@ class Clusters::ClustersController < Clusters::BaseController :enabled, :name, :environment_scope, - :domain, + :base_domain, platform_kubernetes_attributes: [ :api_url, :token, diff --git a/app/models/clusters/cluster.rb b/app/models/clusters/cluster.rb index 2b677961df5..bf339c935cf 100644 --- a/app/models/clusters/cluster.rb +++ b/app/models/clusters/cluster.rb @@ -67,6 +67,8 @@ module Clusters delegate :available?, to: :application_knative, prefix: true, allow_nil: true delegate :external_ip, to: :application_ingress, prefix: true, allow_nil: true + alias_attribute :base_domain, :domain + enum cluster_type: { instance_type: 1, group_type: 2, diff --git a/app/models/project_auto_devops.rb b/app/models/project_auto_devops.rb index d254ec158ca..b6c5c7c4c87 100644 --- a/app/models/project_auto_devops.rb +++ b/app/models/project_auto_devops.rb @@ -26,7 +26,9 @@ class ProjectAutoDevops < ActiveRecord::Base # From 11.8, AUTO_DEVOPS_DOMAIN has been replaced by KUBE_INGRESS_BASE_DOMAIN. # See Clusters::Cluster#predefined_variables and https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/24580 - # for more info. Support for AUTO_DEVOPS_DOMAIN support will be dropped on 12.0. + # for more info. + # Support for AUTO_DEVOPS_DOMAIN support will be dropped on 12.0 on + # https://gitlab.com/gitlab-org/gitlab-ce/issues/52363 def predefined_variables Gitlab::Ci::Variables::Collection.new.tap do |variables| if has_domain? diff --git a/app/views/clusters/clusters/_form.html.haml b/app/views/clusters/clusters/_form.html.haml index 068f14364ec..e0d3b7e1aec 100644 --- a/app/views/clusters/clusters/_form.html.haml +++ b/app/views/clusters/clusters/_form.html.haml @@ -20,7 +20,7 @@ .form-text.text-muted= s_("ClusterIntegration|Choose which of your environments will use this cluster.") - else = text_field_tag :environment_scope, '*', class: 'col-md-6 form-control disabled', placeholder: s_('ClusterIntegration|Environment scope'), disabled: true - - environment_scope_url = 'https://docs.gitlab.com/ee/user/project/clusters/#base-domain' + - environment_scope_url = help_page_path('user/project/clusters', anchor: 'base-domain') - environment_scope_start = ''.html_safe % { url: environment_scope_url } .form-text.text-muted %code * @@ -28,15 +28,15 @@ .form-group %h5= s_('ClusterIntegration|Base domain') - = field.text_field :domain, class: 'col-md-6 form-control js-select-on-focus' + = field.text_field :base_domain, class: 'col-md-6 form-control js-select-on-focus' .form-text.text-muted - if @cluster.application_ingress_external_ip.present? - - auto_devops_url = 'https://docs.gitlab.com/ee/topics/autodevops/' + - auto_devops_url = help_page_path('topics/autodevops/') - auto_devops_start = ''.html_safe % { url: auto_devops_url } = s_('ClusterIntegration|Specifying a domain will allow you to use Auto Review Apps and Auto Deploy stages for %{auto_devops_start}Auto DevOps%{auto_devops_end}. The domain should have a wildcard DNS configured to the Ingress IP Address below.').html_safe % { auto_devops_start: auto_devops_start, auto_devops_end: ''.html_safe } = s_('ClusterIntegration|Alternatively') %code #{@cluster.application_ingress_external_ip}.nip.io - - custom_domain_url = 'https://docs.gitlab.com/ee/user/project/clusters/#pointing-your-dns-at-the-cluster-ip' + - custom_domain_url = help_page_path('user/project/clusters/', anchor: 'pointing-your-dns-at-the-cluster-ip') - custom_domain_start = ''.html_safe % { url: custom_domain_url } = s_('ClusterIntegration| can be used instead of a custom domain. %{custom_domain_start}More information%{custom_domain_end}').html_safe % { custom_domain_start: custom_domain_start, custom_domain_end: ''.html_safe } - else diff --git a/app/views/projects/settings/ci_cd/_autodevops_form.html.haml b/app/views/projects/settings/ci_cd/_autodevops_form.html.haml index c2bbcf8fcaf..d905d015c22 100644 --- a/app/views/projects/settings/ci_cd/_autodevops_form.html.haml +++ b/app/views/projects/settings/ci_cd/_autodevops_form.html.haml @@ -22,7 +22,7 @@ = link_to _('More information'), help_page_path('topics/autodevops/index.md'), target: '_blank' .card-footer.js-extra-settings{ class: @project.auto_devops_enabled? || 'hidden' } %p.settings-message.text-center - - kubernetes_cluster_link = 'https://docs.gitlab.com/ee/user/project/clusters/' + - kubernetes_cluster_link = help_page_path('user/project/clusters/') - kubernetes_cluster_start = ''.html_safe % { url: kubernetes_cluster_link } = s_('CICD|You must add a %{kubernetes_cluster_start}Kubernetes cluster integration%{kubernetes_cluster_end} to this project with a domain in order for your deployment strategy to work correctly.').html_safe % { kubernetes_cluster_start: kubernetes_cluster_start, kubernetes_cluster_end: ''.html_safe } %label.prepend-top-10 diff --git a/changelogs/unreleased/52363-ui-changes-to-cluster-and-ado-pages.yml b/changelogs/unreleased/52363-ui-changes-to-cluster-and-ado-pages.yml index 25f01f95177..eb4851971fb 100644 --- a/changelogs/unreleased/52363-ui-changes-to-cluster-and-ado-pages.yml +++ b/changelogs/unreleased/52363-ui-changes-to-cluster-and-ado-pages.yml @@ -1,5 +1,5 @@ --- -title: Moves domain setting to cluster page +title: Moves domain setting from Auto DevOps to Cluster's page merge_request: 24580 author: type: added diff --git a/db/migrate/20190129165720_migrate_auto_dev_ops_domain_to_cluster_domain.rb b/db/post_migrate/20190204115450_migrate_auto_dev_ops_domain_to_cluster_domain.rb similarity index 100% rename from db/migrate/20190129165720_migrate_auto_dev_ops_domain_to_cluster_domain.rb rename to db/post_migrate/20190204115450_migrate_auto_dev_ops_domain_to_cluster_domain.rb diff --git a/db/schema.rb b/db/schema.rb index 4b6e4992056..68d1240dc73 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20190131122559) do +ActiveRecord::Schema.define(version: 20190204115450) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" diff --git a/lib/gitlab/ci/templates/Auto-DevOps.gitlab-ci.yml b/lib/gitlab/ci/templates/Auto-DevOps.gitlab-ci.yml index a0015c958fe..e369d26f22f 100644 --- a/lib/gitlab/ci/templates/Auto-DevOps.gitlab-ci.yml +++ b/lib/gitlab/ci/templates/Auto-DevOps.gitlab-ci.yml @@ -689,7 +689,7 @@ rollout 100%: --set application.database_url="$DATABASE_URL" \ --set application.secretName="$APPLICATION_SECRET_NAME" \ --set application.secretChecksum="$APPLICATION_SECRET_CHECKSUM" \ - --set service.commonName="le.$AUTO_DEVOPS_DOMAIN" \ + --set service.commonName="le.$KUBE_INGRESS_BASE_DOMAIN" \ --set service.url="$CI_ENVIRONMENT_URL" \ --set service.additionalHosts="$additional_hosts" \ --set replicaCount="$replicas" \ @@ -725,7 +725,7 @@ rollout 100%: --set application.database_url="$DATABASE_URL" \ --set application.secretName="$APPLICATION_SECRET_NAME" \ --set application.secretChecksum="$APPLICATION_SECRET_CHECKSUM" \ - --set service.commonName="le.$AUTO_DEVOPS_DOMAIN" \ + --set service.commonName="le.$KUBE_INGRESS_BASE_DOMAIN" \ --set service.url="$CI_ENVIRONMENT_URL" \ --set service.additionalHosts="$additional_hosts" \ --set replicaCount="$replicas" \ @@ -827,7 +827,7 @@ rollout 100%: # Function to ensure backwards compatibility with AUTO_DEVOPS_DOMAIN function ensure_kube_ingress_base_domain() { if [ -z ${KUBE_INGRESS_BASE_DOMAIN+x} ]; then - export KUBE_INGRESS_BASE_DOMAIN=$AUTO_DEVOPS_DOMAIN + export KUBE_INGRESS_BASE_DOMAIN=$AUTO_DEVOPS_DOMAIN fi } diff --git a/qa/qa/resource/kubernetes_cluster.rb b/qa/qa/resource/kubernetes_cluster.rb index 19c6dc8890d..986b31da528 100644 --- a/qa/qa/resource/kubernetes_cluster.rb +++ b/qa/qa/resource/kubernetes_cluster.rb @@ -9,11 +9,11 @@ module QA :install_helm_tiller, :install_ingress, :install_prometheus, :install_runner, :domain attribute :ingress_ip do - ingress_ip_value + Page::Project::Operations::Kubernetes::Show.perform(&:ingress_ip) end attribute :domain do - "#{ingress_ip_value}.nip.io" + "#{ingress_ip}.nip.io" end def fabricate! @@ -56,12 +56,6 @@ module QA end end end - - private - - def ingress_ip_value - @ingress_ip_value ||= Page::Project::Operations::Kubernetes::Show.perform(&:ingress_ip) - end end end end diff --git a/spec/controllers/groups/clusters_controller_spec.rb b/spec/controllers/groups/clusters_controller_spec.rb index d5a149a57df..360030102e0 100644 --- a/spec/controllers/groups/clusters_controller_spec.rb +++ b/spec/controllers/groups/clusters_controller_spec.rb @@ -436,7 +436,7 @@ describe Groups::ClustersController do cluster: { enabled: false, name: 'my-new-cluster-name', - domain: domain + base_domain: domain } } end diff --git a/spec/features/clusters/cluster_detail_page_spec.rb b/spec/features/clusters/cluster_detail_page_spec.rb index 844008f841a..e3c47fa4721 100644 --- a/spec/features/clusters/cluster_detail_page_spec.rb +++ b/spec/features/clusters/cluster_detail_page_spec.rb @@ -18,7 +18,7 @@ describe 'Clusterable > Show page' do visit cluster_path within '#cluster-integration' do - fill_in('cluster_domain', with: 'test.com') + fill_in('cluster_base_domain', with: 'test.com') click_on 'Save changes' end diff --git a/spec/migrations/migrate_auto_dev_ops_domain_to_cluster_domain_spec.rb b/spec/migrations/migrate_auto_dev_ops_domain_to_cluster_domain_spec.rb index c7fd27588a5..2ffc0e65fee 100644 --- a/spec/migrations/migrate_auto_dev_ops_domain_to_cluster_domain_spec.rb +++ b/spec/migrations/migrate_auto_dev_ops_domain_to_cluster_domain_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true require 'spec_helper' -require Rails.root.join('db', 'migrate', '20190129165720_migrate_auto_dev_ops_domain_to_cluster_domain.rb') +require Rails.root.join('db', 'post_migrate', '20190204115450_migrate_auto_dev_ops_domain_to_cluster_domain.rb') describe MigrateAutoDevOpsDomainToClusterDomain, :migration do include MigrationHelpers::ClusterHelpers