From 6185ce0c84b5674abf4f54576fec44fece571565 Mon Sep 17 00:00:00 2001 From: Jacopo Date: Thu, 21 Dec 2017 22:15:50 +0100 Subject: Rendering of emoji's in Group-Overview Allows rendering of emoji's in the Group Overview. --- app/assets/javascripts/groups/components/group_item.vue | 3 ++- app/assets/javascripts/groups/store/groups_store.js | 2 +- app/serializers/group_child_entity.rb | 9 +++++++++ .../unreleased/40549-render-emoj-in-groups-overview.yml | 5 +++++ spec/features/groups/show_spec.rb | 16 ++++++++++++++++ spec/serializers/group_child_entity_spec.rb | 12 +++++++++++- 6 files changed, 44 insertions(+), 3 deletions(-) create mode 100644 changelogs/unreleased/40549-render-emoj-in-groups-overview.yml diff --git a/app/assets/javascripts/groups/components/group_item.vue b/app/assets/javascripts/groups/components/group_item.vue index 6421547bbde..ad01c261b7f 100644 --- a/app/assets/javascripts/groups/components/group_item.vue +++ b/app/assets/javascripts/groups/components/group_item.vue @@ -139,7 +139,8 @@ export default {
- {{group.description}} + +
😄

') + end + end + it_behaves_like 'group child json' end end -- cgit v1.2.1 From 40264b87af1c8a228a6b943367d6cd06d9f8d812 Mon Sep 17 00:00:00 2001 From: Semyon Pupkov Date: Sun, 24 Dec 2017 17:03:38 +0500 Subject: Move invites spinach test to Rspec https://gitlab.com/gitlab-org/gitlab-ce/issues/23036 --- features/invites.feature | 45 -------------------- features/steps/invites.rb | 80 ----------------------------------- spec/features/invites_spec.rb | 97 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 97 insertions(+), 125 deletions(-) delete mode 100644 features/invites.feature delete mode 100644 features/steps/invites.rb create mode 100644 spec/features/invites_spec.rb diff --git a/features/invites.feature b/features/invites.feature deleted file mode 100644 index dc8eefaeaed..00000000000 --- a/features/invites.feature +++ /dev/null @@ -1,45 +0,0 @@ -Feature: Invites - Background: - Given "John Doe" is owner of group "Owned" - And "John Doe" has invited "user@example.com" to group "Owned" - - Scenario: Viewing invitation when signed out - When I visit the invitation page - Then I should be redirected to the sign in page - And I should see a notice telling me to sign in - - Scenario: Signing in to view invitation - When I visit the invitation page - And I sign in as "Mary Jane" - Then I should be redirected to the invitation page - - Scenario: Viewing invitation when signed in - Given I sign in as "Mary Jane" - And I visit the invitation page - Then I should see the invitation details - And I should see an "Accept invitation" button - And I should see a "Decline" button - - Scenario: Viewing invitation as an existing member - Given I sign in as "John Doe" - And I visit the invitation page - Then I should see a message telling me I'm already a member - - Scenario: Accepting the invitation - Given I sign in as "Mary Jane" - And I visit the invitation page - And I click the "Accept invitation" button - Then I should be redirected to the group page - And I should see a notice telling me I have access - - Scenario: Declining the application when signed in - Given I sign in as "Mary Jane" - And I visit the invitation page - And I click the "Decline" button - Then I should be redirected to the dashboard - And I should see a notice telling me I have declined - - Scenario: Declining the application when signed out - When I visit the invitation's decline page - Then I should be redirected to the sign in page - And I should see a notice telling me I have declined diff --git a/features/steps/invites.rb b/features/steps/invites.rb deleted file mode 100644 index dac972172aa..00000000000 --- a/features/steps/invites.rb +++ /dev/null @@ -1,80 +0,0 @@ -class Spinach::Features::Invites < Spinach::FeatureSteps - include SharedAuthentication - include SharedUser - include SharedGroup - - step '"John Doe" has invited "user@example.com" to group "Owned"' do - user = User.find_by(name: "John Doe") - group = Group.find_by(name: "Owned") - group.add_developer("user@example.com", user) - end - - step 'I visit the invitation page' do - group = Group.find_by(name: "Owned") - invite = group.group_members.invite.last - invite.generate_invite_token! - @raw_invite_token = invite.raw_invite_token - visit invite_path(@raw_invite_token) - end - - step 'I should be redirected to the sign in page' do - expect(current_path).to eq(new_user_session_path) - end - - step 'I should see a notice telling me to sign in' do - expect(page).to have_content "To accept this invitation, sign in" - end - - step 'I should be redirected to the invitation page' do - expect(current_path).to eq(invite_path(@raw_invite_token)) - end - - step 'I should see the invitation details' do - expect(page).to have_content("You have been invited by John Doe to join group Owned as Developer.") - end - - step "I should see a message telling me I'm already a member" do - expect(page).to have_content("However, you are already a member of this group.") - end - - step 'I should see an "Accept invitation" button' do - expect(page).to have_link("Accept invitation") - end - - step 'I should see a "Decline" button' do - expect(page).to have_link("Decline") - end - - step 'I click the "Accept invitation" button' do - page.click_link "Accept invitation" - end - - step 'I should be redirected to the group page' do - group = Group.find_by(name: "Owned") - expect(current_path).to eq(group_path(group)) - end - - step 'I should see a notice telling me I have access' do - expect(page).to have_content("You have been granted Developer access to group Owned.") - end - - step 'I click the "Decline" button' do - page.click_link "Decline" - end - - step 'I should be redirected to the dashboard' do - expect(current_path).to eq(dashboard_projects_path) - end - - step 'I should see a notice telling me I have declined' do - expect(page).to have_content("You have declined the invitation to join group Owned.") - end - - step "I visit the invitation's decline page" do - group = Group.find_by(name: "Owned") - invite = group.group_members.invite.last - invite.generate_invite_token! - @raw_invite_token = invite.raw_invite_token - visit decline_invite_path(@raw_invite_token) - end -end diff --git a/spec/features/invites_spec.rb b/spec/features/invites_spec.rb new file mode 100644 index 00000000000..e4be6193b8b --- /dev/null +++ b/spec/features/invites_spec.rb @@ -0,0 +1,97 @@ +require 'spec_helper' + +describe 'Invites' do + let(:user) { create(:user) } + let(:owner) { create(:user, name: 'John Doe') } + let(:group) { create(:group, name: 'Owned') } + let(:project) { create(:project, :repository, namespace: group) } + let(:invite) { group.group_members.invite.last } + + before do + project.add_master(owner) + group.add_user(owner, Gitlab::Access::OWNER) + group.add_developer('user@example.com', owner) + invite.generate_invite_token! + end + + context 'when signed out' do + before do + visit invite_path(invite.raw_invite_token) + end + + it 'renders sign in page with sign in notice' do + expect(current_path).to eq(new_user_session_path) + expect(page).to have_content('To accept this invitation, sign in') + end + + it 'sign in and redirects to invitation page' do + fill_in 'user_login', with: user.email + fill_in 'user_password', with: user.password + check 'user_remember_me' + click_button 'Sign in' + + expect(current_path).to eq(invite_path(invite.raw_invite_token)) + expect(page).to have_content( + 'You have been invited by John Doe to join group Owned as Developer.' + ) + expect(page).to have_link('Accept invitation') + expect(page).to have_link('Decline') + end + end + + context 'when signed in as an exists member' do + before do + sign_in(owner) + end + + it 'shows message user already a member' do + visit invite_path(invite.raw_invite_token) + expect(page).to have_content('However, you are already a member of this group.') + end + end + + describe 'accepting the invitation' do + before do + sign_in(user) + visit invite_path(invite.raw_invite_token) + end + + it 'grants access and redirects to group page' do + page.click_link 'Accept invitation' + expect(current_path).to eq(group_path(group)) + expect(page).to have_content( + 'You have been granted Developer access to group Owned.' + ) + end + end + + describe 'declining the application' do + context 'when signed in' do + before do + sign_in(user) + visit invite_path(invite.raw_invite_token) + end + + it 'declines application and redirects to dashboard' do + page.click_link 'Decline' + expect(current_path).to eq(dashboard_projects_path) + expect(page).to have_content( + 'You have declined the invitation to join group Owned.' + ) + end + end + + context 'when signed out' do + before do + visit decline_invite_path(invite.raw_invite_token) + end + + it 'declines application and redirects to sign in page' do + expect(current_path).to eq(new_user_session_path) + expect(page).to have_content( + 'You have declined the invitation to join group Owned.' + ) + end + end + end +end -- cgit v1.2.1 From 7a815d7585a8a433359297f37b349604bfbcf2c8 Mon Sep 17 00:00:00 2001 From: Semyon Pupkov Date: Sun, 24 Dec 2017 22:46:30 +0500 Subject: Move explore groups spinach test to RSpec https://gitlab.com/gitlab-org/gitlab-ce/issues/23036 --- features/explore/groups.feature | 105 ----------------------------------- features/steps/explore/groups.rb | 88 ----------------------------- spec/features/explore/groups_spec.rb | 87 +++++++++++++++++++++++++++++ 3 files changed, 87 insertions(+), 193 deletions(-) delete mode 100644 features/explore/groups.feature delete mode 100644 features/steps/explore/groups.rb create mode 100644 spec/features/explore/groups_spec.rb diff --git a/features/explore/groups.feature b/features/explore/groups.feature deleted file mode 100644 index 830810615e0..00000000000 --- a/features/explore/groups.feature +++ /dev/null @@ -1,105 +0,0 @@ -@public -Feature: Explore Groups - Background: - Given group "TestGroup" has private project "Enterprise" - - @javascript - Scenario: I should see group with private and internal projects as user - Given group "TestGroup" has internal project "Internal" - When I sign in as a user - And I visit group "TestGroup" page - Then I should see project "Internal" items - And I should not see project "Enterprise" items - - @javascript - Scenario: I should see group issues for internal project as user - Given group "TestGroup" has internal project "Internal" - When I sign in as a user - And I visit group "TestGroup" issues page - Then I should see project "Internal" items - And I should not see project "Enterprise" items - - @javascript - Scenario: I should see group merge requests for internal project as user - Given group "TestGroup" has internal project "Internal" - When I sign in as a user - And I visit group "TestGroup" merge requests page - Then I should see project "Internal" items - And I should not see project "Enterprise" items - - @javascript - Scenario: I should see group with private, internal and public projects as visitor - Given group "TestGroup" has internal project "Internal" - Given group "TestGroup" has public project "Community" - When I visit group "TestGroup" page - Then I should see project "Community" items - And I should not see project "Internal" items - And I should not see project "Enterprise" items - - @javascript - Scenario: I should see group issues for public project as visitor - Given group "TestGroup" has internal project "Internal" - Given group "TestGroup" has public project "Community" - When I visit group "TestGroup" issues page - Then I should see project "Community" items - And I should not see project "Internal" items - And I should not see project "Enterprise" items - - @javascript - Scenario: I should see group merge requests for public project as visitor - Given group "TestGroup" has internal project "Internal" - Given group "TestGroup" has public project "Community" - When I visit group "TestGroup" merge requests page - Then I should see project "Community" items - And I should not see project "Internal" items - And I should not see project "Enterprise" items - - @javascript - Scenario: I should see group with private, internal and public projects as user - Given group "TestGroup" has internal project "Internal" - Given group "TestGroup" has public project "Community" - When I sign in as a user - And I visit group "TestGroup" page - Then I should see project "Community" items - And I should see project "Internal" items - And I should not see project "Enterprise" items - - @javascript - Scenario: I should see group issues for internal and public projects as user - Given group "TestGroup" has internal project "Internal" - Given group "TestGroup" has public project "Community" - When I sign in as a user - And I visit group "TestGroup" issues page - Then I should see project "Community" items - And I should see project "Internal" items - And I should not see project "Enterprise" items - - @javascript - Scenario: I should see group merge requests for internal and public projects as user - Given group "TestGroup" has internal project "Internal" - Given group "TestGroup" has public project "Community" - When I sign in as a user - And I visit group "TestGroup" merge requests page - Then I should see project "Community" items - And I should see project "Internal" items - And I should not see project "Enterprise" items - - @javascript - Scenario: I should see group with public project in public groups area - Given group "TestGroup" has public project "Community" - When I visit the public groups area - Then I should see group "TestGroup" - - @javascript - Scenario: I should see group with public project in public groups area as user - Given group "TestGroup" has public project "Community" - When I sign in as a user - And I visit the public groups area - Then I should see group "TestGroup" - - @javascript - Scenario: I should see group with internal project in public groups area as user - Given group "TestGroup" has internal project "Internal" - When I sign in as a user - And I visit the public groups area - Then I should see group "TestGroup" diff --git a/features/steps/explore/groups.rb b/features/steps/explore/groups.rb deleted file mode 100644 index 409bf0cb416..00000000000 --- a/features/steps/explore/groups.rb +++ /dev/null @@ -1,88 +0,0 @@ -class Spinach::Features::ExploreGroups < Spinach::FeatureSteps - include SharedAuthentication - include SharedPaths - include SharedGroup - include SharedProject - - step 'group "TestGroup" has private project "Enterprise"' do - group_has_project("TestGroup", "Enterprise", Gitlab::VisibilityLevel::PRIVATE) - end - - step 'group "TestGroup" has internal project "Internal"' do - group_has_project("TestGroup", "Internal", Gitlab::VisibilityLevel::INTERNAL) - end - - step 'group "TestGroup" has public project "Community"' do - group_has_project("TestGroup", "Community", Gitlab::VisibilityLevel::PUBLIC) - end - - step '"John Doe" is owner of group "TestGroup"' do - group = Group.find_by(name: "TestGroup") || create(:group, name: "TestGroup") - user = create(:user, name: "John Doe") - group.add_owner(user) - end - - step 'I visit group "TestGroup" page' do - visit group_path(Group.find_by(name: "TestGroup")) - end - - step 'I visit group "TestGroup" issues page' do - visit issues_group_path(Group.find_by(name: "TestGroup")) - end - - step 'I visit group "TestGroup" merge requests page' do - visit merge_requests_group_path(Group.find_by(name: "TestGroup")) - end - - step 'I visit group "TestGroup" members page' do - visit group_group_members_path(Group.find_by(name: "TestGroup")) - end - - step 'I should not see project "Enterprise" items' do - expect(page).not_to have_content "Enterprise" - end - - step 'I should see project "Internal" items' do - expect(page).to have_content "Internal" - end - - step 'I should not see project "Internal" items' do - expect(page).not_to have_content "Internal" - end - - step 'I should see project "Community" items' do - expect(page).to have_content "Community" - end - - step 'I change filter to Everyone\'s' do - click_link "Everyone's" - end - - step 'I should see group member "John Doe"' do - expect(page).to have_content "John Doe" - end - - protected - - def group_has_project(groupname, projectname, visibility_level) - group = Group.find_by(name: groupname) || create(:group, name: groupname) - project = create(:project, - namespace: group, - name: projectname, - path: "#{groupname}-#{projectname}", - visibility_level: visibility_level - ) - create(:issue, - title: "#{projectname} feature", - project: project - ) - create(:merge_request, - title: "#{projectname} feature implemented", - source_project: project, - target_project: project - ) - create(:closed_issue_event, - project: project - ) - end -end diff --git a/spec/features/explore/groups_spec.rb b/spec/features/explore/groups_spec.rb new file mode 100644 index 00000000000..e4ef47d88dd --- /dev/null +++ b/spec/features/explore/groups_spec.rb @@ -0,0 +1,87 @@ +require 'spec_helper' + +describe 'Explore Groups', :js do + let(:user) { create :user } + let(:group) { create :group } + let!(:private_project) do + create :project, :private, namespace: group do |project| + create(:issue, project: internal_project) + create(:merge_request, source_project: project, target_project: project) + end + end + + let!(:internal_project) do + create :project, :internal, namespace: group do |project| + create(:issue, project: project) + create(:merge_request, source_project: project, target_project: project) + end + end + + let!(:public_project) do + create(:project, :public, namespace: group) do |project| + create(:issue, project: project) + create(:merge_request, source_project: project, target_project: project) + end + end + + shared_examples 'renders public and internal projects' do + it do + visit_page + expect(page).to have_content(public_project.name) + expect(page).to have_content(internal_project.name) + expect(page).not_to have_content(private_project.name) + end + end + + shared_examples 'renders only public project' do + it do + visit_page + expect(page).to have_content(public_project.name) + expect(page).not_to have_content(internal_project.name) + expect(page).not_to have_content(private_project.name) + end + end + + shared_examples 'renders group in public groups area' do + it do + visit explore_groups_path + expect(page).to have_content(group.name) + end + end + + context 'when signed in' do + before do + sign_in(user) + end + + it_behaves_like 'renders public and internal projects' do + subject(:visit_page) { visit group_path(group) } + end + + it_behaves_like 'renders public and internal projects' do + subject(:visit_page) { visit issues_group_path(group) } + end + + it_behaves_like 'renders public and internal projects' do + subject(:visit_page) { visit merge_requests_group_path(group) } + end + + it_behaves_like 'renders group in public groups area' + end + + context 'when signed out' do + it_behaves_like 'renders only public project' do + subject(:visit_page) { visit group_path(group) } + end + + it_behaves_like 'renders only public project' do + subject(:visit_page) { visit issues_group_path(group) } + end + + it_behaves_like 'renders only public project' do + subject(:visit_page) { visit merge_requests_group_path(group) } + end + + it_behaves_like 'renders group in public groups area' + end +end -- cgit v1.2.1 From ac2cb65ab3dc2688b3a1db9de661dc01ed196177 Mon Sep 17 00:00:00 2001 From: Jacob Vosmaer Date: Thu, 4 Jan 2018 18:00:28 +0100 Subject: Remove the Commit#tree method --- app/models/commit.rb | 2 +- lib/gitlab/git/blob.rb | 4 ++-- lib/gitlab/git/commit.rb | 7 +++++-- spec/lib/gitlab/git/blob_spec.rb | 4 ++-- spec/lib/gitlab/git/commit_spec.rb | 1 - spec/models/commit_spec.rb | 1 - 6 files changed, 10 insertions(+), 9 deletions(-) diff --git a/app/models/commit.rb b/app/models/commit.rb index 2be07ca7d3c..39d7f5b159d 100644 --- a/app/models/commit.rb +++ b/app/models/commit.rb @@ -371,7 +371,7 @@ class Commit # # Returns a symbol def uri_type(path) - entry = @raw.tree.path(path) + entry = @raw.rugged_tree_entry(path) if entry[:type] == :blob blob = ::Blob.decorate(Gitlab::Git::Blob.new(name: entry[:name]), @project) blob.image? || blob.video? ? :raw : :blob diff --git a/lib/gitlab/git/blob.rb b/lib/gitlab/git/blob.rb index 228d97a87ab..c4caa306b5d 100644 --- a/lib/gitlab/git/blob.rb +++ b/lib/gitlab/git/blob.rb @@ -154,8 +154,8 @@ module Gitlab end def find_by_rugged(repository, sha, path, limit:) - commit = repository.lookup(sha) - root_tree = commit.tree + rugged_commit = repository.lookup(sha) + root_tree = rugged_commit.tree blob_entry = find_entry_by_path(repository, root_tree.oid, path) diff --git a/lib/gitlab/git/commit.rb b/lib/gitlab/git/commit.rb index 145721dea76..016437b2419 100644 --- a/lib/gitlab/git/commit.rb +++ b/lib/gitlab/git/commit.rb @@ -15,8 +15,6 @@ module Gitlab attr_accessor *SERIALIZE_KEYS # rubocop:disable Lint/AmbiguousOperator - delegate :tree, to: :rugged_commit - def ==(other) return false unless other.is_a?(Gitlab::Git::Commit) @@ -452,6 +450,11 @@ module Gitlab ) end + # Is this the same as Blob.find_entry_by_path ? + def rugged_tree_entry(path) + rugged_commit.tree.path(path) + end + private def init_from_hash(hash) diff --git a/spec/lib/gitlab/git/blob_spec.rb b/spec/lib/gitlab/git/blob_spec.rb index c04a9688503..67838163b05 100644 --- a/spec/lib/gitlab/git/blob_spec.rb +++ b/spec/lib/gitlab/git/blob_spec.rb @@ -146,7 +146,7 @@ describe Gitlab::Git::Blob, seed_helper: true do context 'when sha references a tree' do it 'returns nil' do - tree = Gitlab::Git::Commit.find(repository, 'master').tree + tree = repository.rugged.rev_parse('master^{tree}') blob = Gitlab::Git::Blob.raw(repository, tree.oid) @@ -237,7 +237,7 @@ describe Gitlab::Git::Blob, seed_helper: true do end describe '.batch_lfs_pointers' do - let(:tree_object) { Gitlab::Git::Commit.find(repository, 'master').tree } + let(:tree_object) { repository.rugged.rev_parse('master^{tree}') } let(:non_lfs_blob) do Gitlab::Git::Blob.find( diff --git a/spec/lib/gitlab/git/commit_spec.rb b/spec/lib/gitlab/git/commit_spec.rb index 6d35734d306..6a07a3ca8b8 100644 --- a/spec/lib/gitlab/git/commit_spec.rb +++ b/spec/lib/gitlab/git/commit_spec.rb @@ -55,7 +55,6 @@ describe Gitlab::Git::Commit, seed_helper: true do it { expect(@commit.parents).to eq(@gitlab_parents) } it { expect(@commit.parent_id).to eq(@parents.first.oid) } it { expect(@commit.no_commit_message).to eq("--no commit message") } - it { expect(@commit.tree).to eq(@tree) } after do # Erase the new commit so other tests get the original repo diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index 4f02dc33cd8..817254c7d1e 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -181,7 +181,6 @@ eos it { is_expected.to respond_to(:parents) } it { is_expected.to respond_to(:date) } it { is_expected.to respond_to(:diffs) } - it { is_expected.to respond_to(:tree) } it { is_expected.to respond_to(:id) } it { is_expected.to respond_to(:to_patch) } end -- cgit v1.2.1 From 7f30bb9c29bc1ff0c903a16bbf678db31c7408ec Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Thu, 4 Jan 2018 16:49:15 +0100 Subject: Run background migrations with a minimum interval This adds a minimum interval to BackgroundMigrationWorker, ensuring background migrations of the same class only run once every 5 minutes. This prevents a thundering herd problem where scheduled migrations all run at once due to their delays having been expired (e.g. as the result of a queue being paused for a long time). If a job was recently executed it's rescheduled with a delay that equals the remaining time of the job's lease. This means that if the lease expires in two minutes we only need to wait two minutes, instead of five. Fixes https://gitlab.com/gitlab-org/gitlab-ce/issues/41624 --- app/workers/background_migration_worker.rb | 45 +++++++++++++++++++++- .../unreleased/delay-background-migrations.yml | 5 +++ lib/gitlab/database/migration_helpers.rb | 6 +++ lib/gitlab/exclusive_lease.rb | 11 ++++++ spec/lib/gitlab/database/migration_helpers_spec.rb | 10 ++--- spec/lib/gitlab/exclusive_lease_spec.rb | 15 ++++++++ spec/migrations/normalize_ldap_extern_uids_spec.rb | 6 +-- spec/workers/background_migration_worker_spec.rb | 23 ++++++++++- 8 files changed, 110 insertions(+), 11 deletions(-) create mode 100644 changelogs/unreleased/delay-background-migrations.yml diff --git a/app/workers/background_migration_worker.rb b/app/workers/background_migration_worker.rb index aeb3bc019b9..376703f6319 100644 --- a/app/workers/background_migration_worker.rb +++ b/app/workers/background_migration_worker.rb @@ -1,10 +1,53 @@ class BackgroundMigrationWorker include ApplicationWorker + # The minimum amount of time between processing two jobs of the same migration + # class. + # + # This interval is set to 5 minutes so autovacuuming and other maintenance + # related tasks have plenty of time to clean up after a migration has been + # performed. + MIN_INTERVAL = 5.minutes.to_i + # Performs the background migration. # # See Gitlab::BackgroundMigration.perform for more information. + # + # class_name - The class name of the background migration to run. + # arguments - The arguments to pass to the migration class. def perform(class_name, arguments = []) - Gitlab::BackgroundMigration.perform(class_name, arguments) + should_perform, ttl = perform_and_ttl(class_name) + + if should_perform + Gitlab::BackgroundMigration.perform(class_name, arguments) + else + # If the lease could not be obtained this means either another process is + # running a migration of this class or we ran one recently. In this case + # we'll reschedule the job in such a way that it is picked up again around + # the time the lease expires. + self.class.perform_in(ttl || MIN_INTERVAL, class_name, arguments) + end + end + + def perform_and_ttl(class_name) + if always_perform? + # In test environments `perform_in` will run right away. This can then + # lead to stack level errors in the above `#perform`. To work around this + # we'll just perform the migration right away in the test environment. + [true, nil] + else + lease = lease_for(class_name) + + [lease.try_obtain, lease.ttl] + end + end + + def lease_for(class_name) + Gitlab::ExclusiveLease + .new("#{self.class.name}:#{class_name}", timeout: MIN_INTERVAL) + end + + def always_perform? + Rails.env.test? end end diff --git a/changelogs/unreleased/delay-background-migrations.yml b/changelogs/unreleased/delay-background-migrations.yml new file mode 100644 index 00000000000..aa12591e7d2 --- /dev/null +++ b/changelogs/unreleased/delay-background-migrations.yml @@ -0,0 +1,5 @@ +--- +title: Run background migrations with a minimum interval +merge_request: +author: +type: changed diff --git a/lib/gitlab/database/migration_helpers.rb b/lib/gitlab/database/migration_helpers.rb index 33171f83692..7b35c24d153 100644 --- a/lib/gitlab/database/migration_helpers.rb +++ b/lib/gitlab/database/migration_helpers.rb @@ -842,6 +842,12 @@ into similar problems in the future (e.g. when new tables are created). def queue_background_migration_jobs_by_range_at_intervals(model_class, job_class_name, delay_interval, batch_size: BACKGROUND_MIGRATION_BATCH_SIZE) raise "#{model_class} does not have an ID to use for batch ranges" unless model_class.column_names.include?('id') + # To not overload the worker too much we enforce a minimum interval both + # when scheduling and performing jobs. + if delay_interval < BackgroundMigrationWorker::MIN_INTERVAL + delay_interval = BackgroundMigrationWorker::MIN_INTERVAL + end + model_class.each_batch(of: batch_size) do |relation, index| start_id, end_id = relation.pluck('MIN(id), MAX(id)').first diff --git a/lib/gitlab/exclusive_lease.rb b/lib/gitlab/exclusive_lease.rb index 3f7b42456af..dbb8f317afe 100644 --- a/lib/gitlab/exclusive_lease.rb +++ b/lib/gitlab/exclusive_lease.rb @@ -71,5 +71,16 @@ module Gitlab redis.exists(@redis_shared_state_key) end end + + # Returns the TTL of the Redis key. + # + # This method will return `nil` if no TTL could be obtained. + def ttl + Gitlab::Redis::SharedState.with do |redis| + ttl = redis.ttl(@redis_shared_state_key) + + ttl if ttl.positive? + end + end end end diff --git a/spec/lib/gitlab/database/migration_helpers_spec.rb b/spec/lib/gitlab/database/migration_helpers_spec.rb index 7727a1d81b1..43761c2fe0c 100644 --- a/spec/lib/gitlab/database/migration_helpers_spec.rb +++ b/spec/lib/gitlab/database/migration_helpers_spec.rb @@ -1006,12 +1006,12 @@ describe Gitlab::Database::MigrationHelpers do context 'with batch_size option' do it 'queues jobs correctly' do Sidekiq::Testing.fake! do - model.queue_background_migration_jobs_by_range_at_intervals(User, 'FooJob', 10.seconds, batch_size: 2) + model.queue_background_migration_jobs_by_range_at_intervals(User, 'FooJob', 10.minutes, batch_size: 2) expect(BackgroundMigrationWorker.jobs[0]['args']).to eq(['FooJob', [id1, id2]]) - expect(BackgroundMigrationWorker.jobs[0]['at']).to eq(10.seconds.from_now.to_f) + expect(BackgroundMigrationWorker.jobs[0]['at']).to eq(10.minutes.from_now.to_f) expect(BackgroundMigrationWorker.jobs[1]['args']).to eq(['FooJob', [id3, id3]]) - expect(BackgroundMigrationWorker.jobs[1]['at']).to eq(20.seconds.from_now.to_f) + expect(BackgroundMigrationWorker.jobs[1]['at']).to eq(20.minutes.from_now.to_f) end end end @@ -1019,10 +1019,10 @@ describe Gitlab::Database::MigrationHelpers do context 'without batch_size option' do it 'queues jobs correctly' do Sidekiq::Testing.fake! do - model.queue_background_migration_jobs_by_range_at_intervals(User, 'FooJob', 10.seconds) + model.queue_background_migration_jobs_by_range_at_intervals(User, 'FooJob', 10.minutes) expect(BackgroundMigrationWorker.jobs[0]['args']).to eq(['FooJob', [id1, id3]]) - expect(BackgroundMigrationWorker.jobs[0]['at']).to eq(10.seconds.from_now.to_f) + expect(BackgroundMigrationWorker.jobs[0]['at']).to eq(10.minutes.from_now.to_f) end end end diff --git a/spec/lib/gitlab/exclusive_lease_spec.rb b/spec/lib/gitlab/exclusive_lease_spec.rb index 7322a326b01..6193e177668 100644 --- a/spec/lib/gitlab/exclusive_lease_spec.rb +++ b/spec/lib/gitlab/exclusive_lease_spec.rb @@ -73,4 +73,19 @@ describe Gitlab::ExclusiveLease, :clean_gitlab_redis_shared_state do described_class.new(key, timeout: 3600).try_obtain end end + + describe '#ttl' do + it 'returns the TTL of the Redis key' do + lease = described_class.new('kittens', timeout: 100) + lease.try_obtain + + expect(lease.ttl <= 100).to eq(true) + end + + it 'returns nil when the lease does not exist' do + lease = described_class.new('kittens', timeout: 10) + + expect(lease.ttl).to be_nil + end + end end diff --git a/spec/migrations/normalize_ldap_extern_uids_spec.rb b/spec/migrations/normalize_ldap_extern_uids_spec.rb index 262d7742aaf..56a78f52802 100644 --- a/spec/migrations/normalize_ldap_extern_uids_spec.rb +++ b/spec/migrations/normalize_ldap_extern_uids_spec.rb @@ -27,11 +27,11 @@ describe NormalizeLdapExternUids, :migration, :sidekiq do migrate! expect(BackgroundMigrationWorker.jobs[0]['args']).to eq([described_class::MIGRATION, [1, 2]]) - expect(BackgroundMigrationWorker.jobs[0]['at']).to eq(10.seconds.from_now.to_f) + expect(BackgroundMigrationWorker.jobs[0]['at']).to eq(5.minutes.from_now.to_f) expect(BackgroundMigrationWorker.jobs[1]['args']).to eq([described_class::MIGRATION, [3, 4]]) - expect(BackgroundMigrationWorker.jobs[1]['at']).to eq(20.seconds.from_now.to_f) + expect(BackgroundMigrationWorker.jobs[1]['at']).to eq(10.minutes.from_now.to_f) expect(BackgroundMigrationWorker.jobs[2]['args']).to eq([described_class::MIGRATION, [5, 5]]) - expect(BackgroundMigrationWorker.jobs[2]['at']).to eq(30.seconds.from_now.to_f) + expect(BackgroundMigrationWorker.jobs[2]['at']).to eq(15.minutes.from_now.to_f) expect(BackgroundMigrationWorker.jobs.size).to eq 3 end end diff --git a/spec/workers/background_migration_worker_spec.rb b/spec/workers/background_migration_worker_spec.rb index 1c54cf55fa0..d67e7698635 100644 --- a/spec/workers/background_migration_worker_spec.rb +++ b/spec/workers/background_migration_worker_spec.rb @@ -1,13 +1,32 @@ require 'spec_helper' -describe BackgroundMigrationWorker, :sidekiq do +describe BackgroundMigrationWorker, :sidekiq, :clean_gitlab_redis_shared_state do + let(:worker) { described_class.new } + describe '.perform' do it 'performs a background migration' do expect(Gitlab::BackgroundMigration) .to receive(:perform) .with('Foo', [10, 20]) - described_class.new.perform('Foo', [10, 20]) + worker.perform('Foo', [10, 20]) + end + + it 'reschedules a migration if it was performed recently' do + expect(worker) + .to receive(:always_perform?) + .and_return(false) + + worker.lease_for('Foo').try_obtain + + expect(Gitlab::BackgroundMigration) + .not_to receive(:perform) + + expect(described_class) + .to receive(:perform_in) + .with(a_kind_of(Numeric), 'Foo', [10, 20]) + + worker.perform('Foo', [10, 20]) end end end -- cgit v1.2.1 From fb2ca26f523f7dd4422b4d82f18e92a234ed4c05 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Fri, 5 Jan 2018 23:45:12 +0800 Subject: Remove unused push_code_to_protected_branches --- app/assets/javascripts/users_select.js | 2 -- app/helpers/selects_helper.rb | 1 - app/policies/project_policy.rb | 1 - 3 files changed, 4 deletions(-) diff --git a/app/assets/javascripts/users_select.js b/app/assets/javascripts/users_select.js index 759cc9925f4..f249bd036d6 100644 --- a/app/assets/javascripts/users_select.js +++ b/app/assets/javascripts/users_select.js @@ -541,7 +541,6 @@ function UsersSelect(currentUser, els, options = {}) { options.projectId = $(select).data('project-id'); options.groupId = $(select).data('group-id'); options.showCurrentUser = $(select).data('current-user'); - options.pushCodeToProtectedBranches = $(select).data('push-code-to-protected-branches'); options.authorId = $(select).data('author-id'); options.skipUsers = $(select).data('skip-users'); showNullUser = $(select).data('null-user'); @@ -688,7 +687,6 @@ UsersSelect.prototype.users = function(query, options, callback) { todo_filter: options.todoFilter || null, todo_state_filter: options.todoStateFilter || null, current_user: options.showCurrentUser || null, - push_code_to_protected_branches: options.pushCodeToProtectedBranches || null, author_id: options.authorId || null, skip_users: options.skipUsers || null }, diff --git a/app/helpers/selects_helper.rb b/app/helpers/selects_helper.rb index 1a4f1431bdc..6cefcde558a 100644 --- a/app/helpers/selects_helper.rb +++ b/app/helpers/selects_helper.rb @@ -73,7 +73,6 @@ module SelectsHelper email_user: opts[:email_user] || false, first_user: opts[:first_user] && current_user ? current_user.username : false, current_user: opts[:current_user] || false, - "push-code-to-protected-branches" => opts[:push_code_to_protected_branches], author_id: opts[:author_id] || '', skip_users: opts[:skip_users] ? opts[:skip_users].map(&:id) : nil } diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb index f599eab42f2..1dd8f0a25a9 100644 --- a/app/policies/project_policy.rb +++ b/app/policies/project_policy.rb @@ -241,7 +241,6 @@ class ProjectPolicy < BasePolicy rule { repository_disabled }.policy do prevent :push_code - prevent :push_code_to_protected_branches prevent :download_code prevent :fork_project prevent :read_commit_status -- cgit v1.2.1 From 9ff44c298682fffa0d34ece0677fc4ddeec73f94 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Rodr=C3=ADguez?= Date: Tue, 26 Dec 2017 17:48:07 -0300 Subject: Incorporate RemoteService.FetchInternalRemote Gitaly RPC --- GITALY_SERVER_VERSION | 2 +- lib/gitlab/git/repository.rb | 26 +++++++------- lib/gitlab/gitaly_client/remote_service.rb | 13 +++++++ spec/lib/gitlab/git/repository_spec.rb | 42 +++++++++++++--------- .../gitlab/gitaly_client/remote_service_spec.rb | 13 +++++++ 5 files changed, 66 insertions(+), 30 deletions(-) diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index afed694eede..e40e4fc339c 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -0.65.0 +0.66.0 diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index 37d67b6052c..54c28e45cec 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -1154,23 +1154,13 @@ module Gitlab end def fetch_repository_as_mirror(repository) - remote_name = "tmp-#{SecureRandom.hex}" - - # Notice that this feature flag is not for `fetch_repository_as_mirror` - # as a whole but for the fetching mechanism (file path or gitaly-ssh). - url, env = gitaly_migrate(:fetch_internal) do |is_enabled| + gitaly_migrate(:remote_fetch_internal_remote) do |is_enabled| if is_enabled - repository = RemoteRepository.new(repository) unless repository.is_a?(RemoteRepository) - [GITALY_INTERNAL_URL, repository.fetch_env] + gitaly_remote_client.fetch_internal_remote(repository) else - [repository.path, nil] + rugged_fetch_repository_as_mirror(repository) end end - - add_remote(remote_name, url, mirror_refmap: :all_refs) - fetch_remote(remote_name, env: env) - ensure - remove_remote(remote_name) end def blob_at(sha, path) @@ -1940,6 +1930,16 @@ module Gitlab false end + def rugged_fetch_repository_as_mirror(repository) + remote_name = "tmp-#{SecureRandom.hex}" + repository = RemoteRepository.new(repository) unless repository.is_a?(RemoteRepository) + + add_remote(remote_name, GITALY_INTERNAL_URL, mirror_refmap: :all_refs) + fetch_remote(remote_name, env: repository.fetch_env) + ensure + remove_remote(remote_name) + end + def fetch_remote(remote_name = 'origin', env: nil) run_git(['fetch', remote_name], env: env).last.zero? end diff --git a/lib/gitlab/gitaly_client/remote_service.rb b/lib/gitlab/gitaly_client/remote_service.rb index 9218f6cfd68..559a901b9a3 100644 --- a/lib/gitlab/gitaly_client/remote_service.rb +++ b/lib/gitlab/gitaly_client/remote_service.rb @@ -23,6 +23,19 @@ module Gitlab response.result end + + def fetch_internal_remote(repository) + request = Gitaly::FetchInternalRemoteRequest.new( + repository: @gitaly_repo, + remote_repository: repository.gitaly_repository + ) + + response = GitalyClient.call(@storage, :remote_service, + :fetch_internal_remote, request, + remote_storage: repository.storage) + + response.result + end end end end diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index 0e4292026df..11578e4edd5 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -648,29 +648,39 @@ describe Gitlab::Git::Repository, seed_helper: true do Gitlab::Shell.new.remove_repository(TestEnv.repos_path, 'my_project') end - it 'fetches a repository as a mirror remote' do - subject + shared_examples 'repository mirror fecthing' do + it 'fetches a repository as a mirror remote' do + subject - expect(refs(new_repository.path)).to eq(refs(repository.path)) - end + expect(refs(new_repository.path)).to eq(refs(repository.path)) + end - context 'with keep-around refs' do - let(:sha) { SeedRepo::Commit::ID } - let(:keep_around_ref) { "refs/keep-around/#{sha}" } - let(:tmp_ref) { "refs/tmp/#{SecureRandom.hex}" } + context 'with keep-around refs' do + let(:sha) { SeedRepo::Commit::ID } + let(:keep_around_ref) { "refs/keep-around/#{sha}" } + let(:tmp_ref) { "refs/tmp/#{SecureRandom.hex}" } - before do - repository.rugged.references.create(keep_around_ref, sha, force: true) - repository.rugged.references.create(tmp_ref, sha, force: true) - end + before do + repository.rugged.references.create(keep_around_ref, sha, force: true) + repository.rugged.references.create(tmp_ref, sha, force: true) + end - it 'includes the temporary and keep-around refs' do - subject + it 'includes the temporary and keep-around refs' do + subject - expect(refs(new_repository.path)).to include(keep_around_ref) - expect(refs(new_repository.path)).to include(tmp_ref) + expect(refs(new_repository.path)).to include(keep_around_ref) + expect(refs(new_repository.path)).to include(tmp_ref) + end end end + + context 'with gitaly enabled' do + it_behaves_like 'repository mirror fecthing' + end + + context 'with gitaly enabled', :skip_gitaly_mock do + it_behaves_like 'repository mirror fecthing' + end end describe '#remote_tags' do diff --git a/spec/lib/gitlab/gitaly_client/remote_service_spec.rb b/spec/lib/gitlab/gitaly_client/remote_service_spec.rb index 69c6f054016..9d540446532 100644 --- a/spec/lib/gitlab/gitaly_client/remote_service_spec.rb +++ b/spec/lib/gitlab/gitaly_client/remote_service_spec.rb @@ -31,4 +31,17 @@ describe Gitlab::GitalyClient::RemoteService do expect(client.remove_remote(remote_name)).to be(true) end end + + describe '#fetch_internal_remote' do + let(:remote_repository) { Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') } + + it 'sends an fetch_internal_remote message and returns the result value' do + expect_any_instance_of(Gitaly::RemoteService::Stub) + .to receive(:fetch_internal_remote) + .with(gitaly_request_with_path(storage_name, relative_path), kind_of(Hash)) + .and_return(double(result: true)) + + expect(client.fetch_internal_remote(remote_repository)).to be(true) + end + end end -- cgit v1.2.1 From 819fc98fed227487b0a273ee294e374e7457782b Mon Sep 17 00:00:00 2001 From: Tiago Botelho Date: Wed, 3 Jan 2018 16:01:46 +0000 Subject: Protected branch is now created for default branch on import --- app/models/project.rb | 22 ++++++++++++ app/services/git_push_service.rb | 19 +--------- app/services/protected_branches/create_service.rb | 4 +-- ...nch-is-not-set-for-default-branch-on-import.yml | 5 +++ spec/models/project_spec.rb | 42 ++++++++++++++++++++++ .../protected_branches/create_service_spec.rb | 16 +++++++++ spec/workers/repository_import_worker_spec.rb | 1 + 7 files changed, 89 insertions(+), 20 deletions(-) create mode 100644 changelogs/unreleased/3968-protected-branch-is-not-set-for-default-branch-on-import.yml diff --git a/app/models/project.rb b/app/models/project.rb index 9c0bbf697e2..ff9418ea70c 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1459,6 +1459,7 @@ class Project < ActiveRecord::Base import_finish remove_import_jid update_project_counter_caches + after_create_default_branch end def update_project_counter_caches @@ -1472,6 +1473,27 @@ class Project < ActiveRecord::Base end end + def after_create_default_branch + return unless default_branch + + # Ensure HEAD points to the default branch in case it is not master + change_head(default_branch) + + if current_application_settings.default_branch_protection != Gitlab::Access::PROTECTION_NONE && !ProtectedBranch.protected?(self, default_branch) + params = { + name: default_branch, + push_access_levels_attributes: [{ + access_level: current_application_settings.default_branch_protection == Gitlab::Access::PROTECTION_DEV_CAN_PUSH ? Gitlab::Access::DEVELOPER : Gitlab::Access::MASTER + }], + merge_access_levels_attributes: [{ + access_level: current_application_settings.default_branch_protection == Gitlab::Access::PROTECTION_DEV_CAN_MERGE ? Gitlab::Access::DEVELOPER : Gitlab::Access::MASTER + }] + } + + ProtectedBranches::CreateService.new(self, creator, params).execute(skip_authorization: true) + end + end + def remove_import_jid return unless import_jid diff --git a/app/services/git_push_service.rb b/app/services/git_push_service.rb index bb61136e33b..e6fd193ffb3 100644 --- a/app/services/git_push_service.rb +++ b/app/services/git_push_service.rb @@ -154,24 +154,7 @@ class GitPushService < BaseService offset = [@push_commits_count - PROCESS_COMMIT_LIMIT, 0].max @push_commits = project.repository.commits(params[:newrev], offset: offset, limit: PROCESS_COMMIT_LIMIT) - # Ensure HEAD points to the default branch in case it is not master - project.change_head(branch_name) - - # Set protection on the default branch if configured - if current_application_settings.default_branch_protection != PROTECTION_NONE && !ProtectedBranch.protected?(@project, @project.default_branch) - - params = { - name: @project.default_branch, - push_access_levels_attributes: [{ - access_level: current_application_settings.default_branch_protection == PROTECTION_DEV_CAN_PUSH ? Gitlab::Access::DEVELOPER : Gitlab::Access::MASTER - }], - merge_access_levels_attributes: [{ - access_level: current_application_settings.default_branch_protection == PROTECTION_DEV_CAN_MERGE ? Gitlab::Access::DEVELOPER : Gitlab::Access::MASTER - }] - } - - ProtectedBranches::CreateService.new(@project, current_user, params).execute - end + @project.after_create_default_branch end def build_push_data diff --git a/app/services/protected_branches/create_service.rb b/app/services/protected_branches/create_service.rb index a84e335340d..6212fd69077 100644 --- a/app/services/protected_branches/create_service.rb +++ b/app/services/protected_branches/create_service.rb @@ -2,8 +2,8 @@ module ProtectedBranches class CreateService < BaseService attr_reader :protected_branch - def execute - raise Gitlab::Access::AccessDeniedError unless can?(current_user, :admin_project, project) + def execute(skip_authorization: false) + raise Gitlab::Access::AccessDeniedError unless skip_authorization || can?(current_user, :admin_project, project) project.protected_branches.create(params) end diff --git a/changelogs/unreleased/3968-protected-branch-is-not-set-for-default-branch-on-import.yml b/changelogs/unreleased/3968-protected-branch-is-not-set-for-default-branch-on-import.yml new file mode 100644 index 00000000000..e972ac6d54a --- /dev/null +++ b/changelogs/unreleased/3968-protected-branch-is-not-set-for-default-branch-on-import.yml @@ -0,0 +1,5 @@ +--- +title: Protected branch is now created for default branch on import +merge_request: 16198 +author: +type: fixed diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index cea22bbd184..9b6646dcae1 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -3072,9 +3072,51 @@ describe Project do expect(project).to receive(:import_finish) expect(project).to receive(:update_project_counter_caches) expect(project).to receive(:remove_import_jid) + expect(project).to receive(:after_create_default_branch) project.after_import end + + context 'branch protection' do + let(:project) { create(:project, :repository) } + + it 'does not protect when branch protection is disabled' do + stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_NONE) + + project.after_import + + expect(project.protected_branches).to be_empty + end + + it "gives developer access to push when branch protection is set to 'developers can push'" do + stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_DEV_CAN_PUSH) + + project.after_import + + expect(project.protected_branches).not_to be_empty + expect(project.default_branch).to eq(project.protected_branches.first.name) + expect(project.protected_branches.first.push_access_levels.map(&:access_level)).to eq([Gitlab::Access::DEVELOPER]) + end + + it "gives developer access to merge when branch protection is set to 'developers can merge'" do + stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_DEV_CAN_MERGE) + + project.after_import + + expect(project.protected_branches).not_to be_empty + expect(project.default_branch).to eq(project.protected_branches.first.name) + expect(project.protected_branches.first.merge_access_levels.map(&:access_level)).to eq([Gitlab::Access::DEVELOPER]) + end + + it 'protects default branch' do + project.after_import + + expect(project.protected_branches).not_to be_empty + expect(project.default_branch).to eq(project.protected_branches.first.name) + expect(project.protected_branches.first.push_access_levels.map(&:access_level)).to eq([Gitlab::Access::MASTER]) + expect(project.protected_branches.first.merge_access_levels.map(&:access_level)).to eq([Gitlab::Access::MASTER]) + end + end end describe '#update_project_counter_caches' do diff --git a/spec/services/protected_branches/create_service_spec.rb b/spec/services/protected_branches/create_service_spec.rb index 835e83d6dba..53b3e5e365d 100644 --- a/spec/services/protected_branches/create_service_spec.rb +++ b/spec/services/protected_branches/create_service_spec.rb @@ -19,5 +19,21 @@ describe ProtectedBranches::CreateService do expect(project.protected_branches.last.push_access_levels.map(&:access_level)).to eq([Gitlab::Access::MASTER]) expect(project.protected_branches.last.merge_access_levels.map(&:access_level)).to eq([Gitlab::Access::MASTER]) end + + context 'when user does not have permission' do + let(:user) { create(:user) } + + before do + project.add_developer(user) + end + + it 'creates a new protected branch if we skip authorization step' do + expect { service.execute(skip_authorization: true) }.to change(ProtectedBranch, :count).by(1) + end + + it 'raises Gitlab::Access:AccessDeniedError' do + expect { service.execute }.to raise_error(Gitlab::Access::AccessDeniedError) + end + end end end diff --git a/spec/workers/repository_import_worker_spec.rb b/spec/workers/repository_import_worker_spec.rb index 85ac14eb347..7274a9f00f9 100644 --- a/spec/workers/repository_import_worker_spec.rb +++ b/spec/workers/repository_import_worker_spec.rb @@ -32,6 +32,7 @@ describe RepositoryImportWorker do expect_any_instance_of(Projects::ImportService).to receive(:execute) .and_return({ status: :ok }) + expect_any_instance_of(Project).to receive(:after_import).and_call_original expect_any_instance_of(Repository).to receive(:expire_emptiness_caches) expect_any_instance_of(Project).to receive(:import_finish) -- cgit v1.2.1 From 5b2ca1c66c3a69a1177c0d2f62c208bdce7a81c0 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Fri, 24 Nov 2017 18:31:13 +0900 Subject: Migrate existing data from KubernetesService to Clusters::Platforms::Kubernetes --- ...rnetes_service_to_new_clusters_architectures.rb | 104 +++++++++++++++++++++ 1 file changed, 104 insertions(+) create mode 100644 db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb diff --git a/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb b/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb new file mode 100644 index 00000000000..dde69058523 --- /dev/null +++ b/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb @@ -0,0 +1,104 @@ +class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migration + DOWNTIME = false + DEFAULT_KUBERNETES_SERVICE_CLUSTER_NAME = 'KubernetesService' + + class Cluster < ActiveRecord::Base + self.table_name = 'clusters' + + has_many :cluster_projects, class_name: 'ClustersProject' + has_many :projects, through: :cluster_projects, class_name: 'Project' + has_one :provider_gcp, class_name: 'ProvidersGcp' + has_one :platform_kubernetes, class_name: 'PlatformsKubernetes' + + attr_encrypted :token, + mode: :per_attribute_iv, + key: Gitlab::Application.secrets.db_key_base, + algorithm: 'aes-256-cbc' + + accepts_nested_attributes_for :provider_gcp + accepts_nested_attributes_for :platform_kubernetes + + enum platform_type: { + kubernetes: 1 + } + + enum provider_type: { + user: 0, + gcp: 1 + } + end + + class Project < ActiveRecord::Base + self.table_name = 'projects' + + has_one :cluster_project, class_name: 'ClustersProject' + has_one :cluster, through: :cluster_project, class_name: 'Cluster' + end + + class Service < ActiveRecord::Base + include EachBatch + + self.table_name = 'services' + + belongs_to :project, class_name: 'Project' + + # When users create a cluster, KubernetesService is automatically synchronized + # with Platforms::Kubernetes due to delegate Kubernetes specific logic. + # We only target unmanaged KubernetesService records. + scope :unmanaged_kubernetes_service, -> do + joins( + 'INNER JOIN projects ON projects.id = services.project_id' \ + 'INNER JOIN cluster_projects ON projects.id = cluster_projects.project_id' \ + 'INNER JOIN clusters ON cluster_projects.cluster_id = clusters.id' \ + 'INNER JOIN cluster_platforms_kubernetes ON cluster_platforms_kubernetes.cluster_id = clusters.id') + .where( + "services.category = 'deployment' AND services.type = 'KubernetesService'" \ + "AND (" \ + " cluster_projects.project_id IS NULL" \ + " OR" \ + " services.properties NOT LIKE CONCAT('%', cluster_platforms_kubernetes.api_url, '%')" \ + ")") + end + end + + class ClustersProject < ActiveRecord::Base + self.table_name = 'cluster_projects' + + belongs_to :cluster, class_name: 'Cluster' + belongs_to :project, class_name: 'Project' + end + + class ProvidersGcp < ActiveRecord::Base + self.table_name = 'cluster_providers_gcp' + end + + class PlatformsKubernetes < ActiveRecord::Base + self.table_name = 'cluster_platforms_kubernetes' + end + + def up + Service.unmanaged_kubernetes_service + .find_each(batch_size: 1) do |kubernetes_service| + Cluster.create( + enabled: kubernetes_service.active, + user_id: nil, # KubernetesService doesn't have + name: DEFAULT_KUBERNETES_SERVICE_CLUSTER_NAME, + provider_type: Cluster.provider_types[:user], + platform_type: Cluster.platform_types[:kubernetes], + projects: [kubernetes_service.project], + platform_kubernetes_attributes: { + api_url: kubernetes_service.api_url, + ca_cert: kubernetes_service.ca_pem, + namespace: kubernetes_service.namespace, + username: nil, # KubernetesService doesn't have + encrypted_password: nil, # KubernetesService doesn't have + encrypted_password_iv: nil, # KubernetesService doesn't have + token: kubernetes_service.token # encrypted_token and encrypted_token_iv + } ) + end + end + + def down + # noop + end +end -- cgit v1.2.1 From e4745492821440b47a48b75e8786d049fde50fca Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Wed, 29 Nov 2017 02:38:55 +0900 Subject: Add test. Disable KubernetesService when migrated --- ...rnetes_service_to_new_clusters_architectures.rb | 23 +-- ...s_service_to_new_clusters_architectures_spec.rb | 189 +++++++++++++++++++++ 2 files changed, 201 insertions(+), 11 deletions(-) create mode 100644 spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb diff --git a/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb b/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb index dde69058523..e1e02c2852a 100644 --- a/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb +++ b/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb @@ -46,18 +46,16 @@ class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migrati # with Platforms::Kubernetes due to delegate Kubernetes specific logic. # We only target unmanaged KubernetesService records. scope :unmanaged_kubernetes_service, -> do - joins( - 'INNER JOIN projects ON projects.id = services.project_id' \ - 'INNER JOIN cluster_projects ON projects.id = cluster_projects.project_id' \ - 'INNER JOIN clusters ON cluster_projects.cluster_id = clusters.id' \ + joins('INNER JOIN projects ON projects.id = services.project_id ' \ + 'INNER JOIN cluster_projects ON projects.id = cluster_projects.project_id ' \ + 'INNER JOIN clusters ON cluster_projects.cluster_id = clusters.id ' \ 'INNER JOIN cluster_platforms_kubernetes ON cluster_platforms_kubernetes.cluster_id = clusters.id') - .where( - "services.category = 'deployment' AND services.type = 'KubernetesService'" \ - "AND (" \ - " cluster_projects.project_id IS NULL" \ - " OR" \ - " services.properties NOT LIKE CONCAT('%', cluster_platforms_kubernetes.api_url, '%')" \ - ")") + .where("services.category = 'deployment' AND services.type = 'KubernetesService' " \ + "AND ( " \ + " cluster_projects.project_id IS NULL " \ + " OR " \ + " services.properties NOT LIKE CONCAT('%', cluster_platforms_kubernetes.api_url, '%') " \ + ") ") end end @@ -95,6 +93,9 @@ class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migrati encrypted_password_iv: nil, # KubernetesService doesn't have token: kubernetes_service.token # encrypted_token and encrypted_token_iv } ) + + # Disable the service, so that new cluster archetecture is going to be used + kubernetes_service.updated(active: false) end end diff --git a/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb b/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb new file mode 100644 index 00000000000..5621f2689fa --- /dev/null +++ b/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb @@ -0,0 +1,189 @@ +require 'spec_helper' +require Rails.root.join('db', 'post_migrate', '20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb') + +describe MigrateKubernetesServiceToNewClustersArchitectures, :migration do + context 'when user configured kubernetes from CI/CD > Clusters' do + let(:project) { create(:project) } + let(:user) { create(:user) } + + # Platforms::Kubernetes (New archtecture) + let!(:cluster) do + create(:cluster, + projects: [project], + user: user, + provider_type: :gcp, + platform_type: :kubernetes, + provider_gcp: provider_gcp, + platform_kubernetes: platform_kubernetes) + end + + let(:provider_gcp) { create(:cluster_provider_gcp, :created) } + let(:platform_kubernetes) { create(:cluster_platform_kubernetes, :configured) } + + # KubernetesService (Automatically synchronized when Platforms::Kubernetes created) + let!(:kubernetes_service) { create(:kubernetes_service, project: project) } + + context 'when user is using the cluster' do + it 'migrates' do + expect{ migrate! }.not_to change { Clusters::Cluster.count } + expect(cluster).to be_active + expect(kubernetes_service).not_to be_active + end + end + + context 'when user disabled cluster' do + before do + disable_cluster! + end + + context 'when user configured kubernetes from Integration > Kubernetes' do + before do + kubernetes_service.update( + active: true, + api_url: 'http://new.kube.com', + ca_pem: nil, + token: 'z' * 40).reload + end + + context 'when user is using the kubernetes service' do + it 'migrates' do + expect{ migrate! }.to change { Clusters::Cluster.count }.by(1) + + Clusters::Cluster.last.tap do |c| + expect(c).to be_active + expect(c.platform_kubernetes.api_url).to eq(kubernetes_service.api_url) + expect(c.platform_kubernetes.ca_pem).to eq(kubernetes_service.ca_pem) + expect(c.platform_kubernetes.token).to eq(kubernetes_service.token) + end + + expect(kubernetes_service).not_to be_active + end + end + + context 'when user stopped using the kubernetes service' do + before do + kubernetes_service.update(active: false) + end + + it 'migrates' do + expect{ migrate! }.to change { Clusters::Cluster.count }.by(1) + + Clusters::Cluster.last.tap do |c| + expect(c).not_to be_active + expect(c.platform_kubernetes.api_url).to eq(kubernetes_service.api_url) + expect(c.platform_kubernetes.ca_pem).to eq(kubernetes_service.ca_pem) + expect(c.platform_kubernetes.token).to eq(kubernetes_service.token) + end + + expect(kubernetes_service).not_to be_active + end + end + end + end + + context 'when user deleted cluster' do + before do + destory_cluster! + end + + context 'when user configured kubernetes from Integration > Kubernetes' do + let!(:new_kubernetes_service) do + project.create_kubernetes_service( + active: true, + api_url: 'http://123.123.123.123', + ca_pem: nil, + token: 'a' * 40) + end + + context 'when user is using the kubernetes service' do + it 'migrates' do + expect{ migrate! }.to change { Clusters::Cluster.count }.by(1) + + Clusters::Cluster.last.tap do |c| + expect(c).to be_active + expect(c.platform_kubernetes.api_url).to eq(new_kubernetes_service.api_url) + expect(c.platform_kubernetes.ca_pem).to eq(new_kubernetes_service.ca_pem) + expect(c.platform_kubernetes.token).to eq(new_kubernetes_service.token) + end + + expect(new_kubernetes_service).not_to be_active + end + end + + context 'when user stopped using the kubernetes service' do + before do + new_kubernetes_service.update(active: false) + end + + it 'migrates' do + expect{ migrate! }.to change { Clusters::Cluster.count }.by(1) + + Clusters::Cluster.last.tap do |c| + expect(c).not_to be_active + expect(c.platform_kubernetes.api_url).to eq(new_kubernetes_service.api_url) + expect(c.platform_kubernetes.ca_pem).to eq(new_kubernetes_service.ca_pem) + expect(c.platform_kubernetes.token).to eq(new_kubernetes_service.token) + end + + expect(new_kubernetes_service).not_to be_active + end + end + end + end + end + + context 'when user configured kubernetes from Integration > Kubernetes' do + let(:project) { create(:project) } + let!(:kubernetes_service) { create(:kubernetes_service, project: project) } + + context 'when user is using the kubernetes service' do + it 'migrates' do + expect{ migrate! }.to change { Clusters::Cluster.count }.by(1) + + Clusters::Cluster.last.tap do |c| + expect(c).to be_active + expect(c.platform_kubernetes.api_url).to eq(kubernetes_service.api_url) + expect(c.platform_kubernetes.ca_pem).to eq(kubernetes_service.ca_pem) + expect(c.platform_kubernetes.token).to eq(kubernetes_service.token) + end + + expect(kubernetes_service).not_to be_active + end + end + + context 'when user stopped using the kubernetes service' do + before do + kubernetes_service.update(active: false) + end + + it 'migrates' do + expect{ migrate! }.to change { Clusters::Cluster.count }.by(1) + + Clusters::Cluster.last.tap do |c| + expect(c).not_to be_active + expect(c.platform_kubernetes.api_url).to eq(kubernetes_service.api_url) + expect(c.platform_kubernetes.ca_pem).to eq(kubernetes_service.ca_pem) + expect(c.platform_kubernetes.token).to eq(kubernetes_service.token) + end + + expect(kubernetes_service).not_to be_active + end + end + end + + context 'when nothing is configured' do + it 'migrates' do + expect{ migrate! }.not_to change { Clusters::Cluster.count } + end + end + + def disable_cluster! + cluster.update!(enabled: false) + kubernetes_service.update!(active: false) + end + + def destory_cluster! + cluster.destroy! + kubernetes_service.destroy! + end +end -- cgit v1.2.1 From b9fbfe5a6b21bed39010224fd012366d3d39b117 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Thu, 14 Dec 2017 00:17:56 +0900 Subject: Fix unmanaged_kubernetes_service scope for multiple clusters --- ...rnetes_service_to_new_clusters_architectures.rb | 44 +++++++++++++--------- 1 file changed, 27 insertions(+), 17 deletions(-) diff --git a/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb b/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb index e1e02c2852a..4b9e3094f94 100644 --- a/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb +++ b/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb @@ -1,13 +1,12 @@ class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migration DOWNTIME = false - DEFAULT_KUBERNETES_SERVICE_CLUSTER_NAME = 'KubernetesService' + DEFAULT_KUBERNETES_SERVICE_CLUSTER_NAME = 'KubernetesService'.freeze class Cluster < ActiveRecord::Base self.table_name = 'clusters' has_many :cluster_projects, class_name: 'ClustersProject' has_many :projects, through: :cluster_projects, class_name: 'Project' - has_one :provider_gcp, class_name: 'ProvidersGcp' has_one :platform_kubernetes, class_name: 'PlatformsKubernetes' attr_encrypted :token, @@ -15,7 +14,6 @@ class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migrati key: Gitlab::Application.secrets.db_key_base, algorithm: 'aes-256-cbc' - accepts_nested_attributes_for :provider_gcp accepts_nested_attributes_for :platform_kubernetes enum platform_type: { @@ -31,8 +29,8 @@ class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migrati class Project < ActiveRecord::Base self.table_name = 'projects' - has_one :cluster_project, class_name: 'ClustersProject' - has_one :cluster, through: :cluster_project, class_name: 'Cluster' + has_many :cluster_projects, class_name: 'ClustersProject' + has_many :clusters, through: :cluster_projects, class_name: 'Cluster' end class Service < ActiveRecord::Base @@ -42,20 +40,30 @@ class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migrati belongs_to :project, class_name: 'Project' + # 10.1 ~ 10.2 # When users create a cluster, KubernetesService is automatically synchronized # with Platforms::Kubernetes due to delegate Kubernetes specific logic. # We only target unmanaged KubernetesService records. + # + # 10.3 + # We no longer create KubernetesService because Platforms::Kubernetes has the specific logic. + # + # "unmanaged" means "unmanaged by Platforms::Kubernetes(New archetecture)" + # + # "cluster_projects.project_id IS NULL" -> it's not copied from KubernetesService + # "services.properties NOT LIKE CONCAT('%', cluster_platforms_kubernetes.api_url, '%')" -> KubernetesService has unique configuration which is not included in Platforms::Kubernetes scope :unmanaged_kubernetes_service, -> do - joins('INNER JOIN projects ON projects.id = services.project_id ' \ - 'INNER JOIN cluster_projects ON projects.id = cluster_projects.project_id ' \ - 'INNER JOIN clusters ON cluster_projects.cluster_id = clusters.id ' \ - 'INNER JOIN cluster_platforms_kubernetes ON cluster_platforms_kubernetes.cluster_id = clusters.id') - .where("services.category = 'deployment' AND services.type = 'KubernetesService' " \ - "AND ( " \ - " cluster_projects.project_id IS NULL " \ - " OR " \ - " services.properties NOT LIKE CONCAT('%', cluster_platforms_kubernetes.api_url, '%') " \ - ") ") + joins('INNER JOIN projects ON projects.id = services.project_id') + .where("services.category = 'deployment'") + .where("services.type = 'KubernetesService'") + .where("services.template = FALSE") + .where("NOT EXISTS (?)", + PlatformsKubernetes + .joins('INNER JOIN cluster_projects ON cluster_projects.project_id = projects.id') + .where('cluster_projects.cluster_id = cluster_platforms_kubernetes.cluster_id') + .where("services.properties LIKE CONCAT('%', cluster_platforms_kubernetes.api_url, '%')") + .select('1') ) + .order('services.project_id') end end @@ -94,8 +102,10 @@ class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migrati token: kubernetes_service.token # encrypted_token and encrypted_token_iv } ) - # Disable the service, so that new cluster archetecture is going to be used - kubernetes_service.updated(active: false) + # Disable the KubernetesService. Platforms::Kubernetes will be used from next time. + kubernetes_service.active = false + kubernetes_service.propaties.merge!( { migrated: true } ) + kubernetes_service.save! end end -- cgit v1.2.1 From 40c6af546e3a941f829bf91435e848c22bf2aed8 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Fri, 15 Dec 2017 02:56:05 +0900 Subject: Fix migration file typos and reorder Table definition --- ...rnetes_service_to_new_clusters_architectures.rb | 46 ++++++++++------------ 1 file changed, 21 insertions(+), 25 deletions(-) diff --git a/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb b/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb index 4b9e3094f94..7c7e5046ec8 100644 --- a/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb +++ b/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb @@ -2,6 +2,13 @@ class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migrati DOWNTIME = false DEFAULT_KUBERNETES_SERVICE_CLUSTER_NAME = 'KubernetesService'.freeze + class Project < ActiveRecord::Base + self.table_name = 'projects' + + has_many :cluster_projects, class_name: 'ClustersProject' + has_many :clusters, through: :cluster_projects, class_name: 'Cluster' + end + class Cluster < ActiveRecord::Base self.table_name = 'clusters' @@ -9,11 +16,6 @@ class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migrati has_many :projects, through: :cluster_projects, class_name: 'Project' has_one :platform_kubernetes, class_name: 'PlatformsKubernetes' - attr_encrypted :token, - mode: :per_attribute_iv, - key: Gitlab::Application.secrets.db_key_base, - algorithm: 'aes-256-cbc' - accepts_nested_attributes_for :platform_kubernetes enum platform_type: { @@ -26,11 +28,20 @@ class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migrati } end - class Project < ActiveRecord::Base - self.table_name = 'projects' + class ClustersProject < ActiveRecord::Base + self.table_name = 'cluster_projects' - has_many :cluster_projects, class_name: 'ClustersProject' - has_many :clusters, through: :cluster_projects, class_name: 'Cluster' + belongs_to :cluster, class_name: 'Cluster' + belongs_to :project, class_name: 'Project' + end + + class PlatformsKubernetes < ActiveRecord::Base + self.table_name = 'cluster_platforms_kubernetes' + + attr_encrypted :token, + mode: :per_attribute_iv, + key: Gitlab::Application.secrets.db_key_base, + algorithm: 'aes-256-cbc' end class Service < ActiveRecord::Base @@ -67,21 +78,6 @@ class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migrati end end - class ClustersProject < ActiveRecord::Base - self.table_name = 'cluster_projects' - - belongs_to :cluster, class_name: 'Cluster' - belongs_to :project, class_name: 'Project' - end - - class ProvidersGcp < ActiveRecord::Base - self.table_name = 'cluster_providers_gcp' - end - - class PlatformsKubernetes < ActiveRecord::Base - self.table_name = 'cluster_platforms_kubernetes' - end - def up Service.unmanaged_kubernetes_service .find_each(batch_size: 1) do |kubernetes_service| @@ -104,7 +100,7 @@ class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migrati # Disable the KubernetesService. Platforms::Kubernetes will be used from next time. kubernetes_service.active = false - kubernetes_service.propaties.merge!( { migrated: true } ) + kubernetes_service.properties.merge!( { migrated: true } ) kubernetes_service.save! end end -- cgit v1.2.1 From 27111e2940115be9c7c97648a95b80d9fefbf722 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Fri, 15 Dec 2017 02:56:23 +0900 Subject: Restructure spec --- ...s_service_to_new_clusters_architectures_spec.rb | 389 ++++++++++++--------- 1 file changed, 229 insertions(+), 160 deletions(-) diff --git a/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb b/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb index 5621f2689fa..0f270dd37da 100644 --- a/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb +++ b/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb @@ -2,188 +2,257 @@ require 'spec_helper' require Rails.root.join('db', 'post_migrate', '20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb') describe MigrateKubernetesServiceToNewClustersArchitectures, :migration do - context 'when user configured kubernetes from CI/CD > Clusters' do - let(:project) { create(:project) } - let(:user) { create(:user) } - - # Platforms::Kubernetes (New archtecture) - let!(:cluster) do - create(:cluster, - projects: [project], - user: user, - provider_type: :gcp, - platform_type: :kubernetes, - provider_gcp: provider_gcp, - platform_kubernetes: platform_kubernetes) - end - - let(:provider_gcp) { create(:cluster_provider_gcp, :created) } - let(:platform_kubernetes) { create(:cluster_platform_kubernetes, :configured) } - - # KubernetesService (Automatically synchronized when Platforms::Kubernetes created) - let!(:kubernetes_service) { create(:kubernetes_service, project: project) } - - context 'when user is using the cluster' do - it 'migrates' do - expect{ migrate! }.not_to change { Clusters::Cluster.count } - expect(cluster).to be_active - expect(kubernetes_service).not_to be_active - end - end - - context 'when user disabled cluster' do - before do - disable_cluster! - end - - context 'when user configured kubernetes from Integration > Kubernetes' do - before do - kubernetes_service.update( - active: true, - api_url: 'http://new.kube.com', - ca_pem: nil, - token: 'z' * 40).reload + context 'when unique KubernetesService exists' do + shared_examples 'KubernetesService migration' do + let(:sample_num) { 2 } + let(:projects) { create_list(:project, sample_num) } + + let!(:kubernetes_services) do + projects.map do |project| + create(:kubernetes_service, + project: project, + active: active, + api_url: "https://kubernetes#{project.id}.com", + token: defined?(token) ? token : "token#{project.id}", + ca_pem: "ca_pem#{project.id}") end + end - context 'when user is using the kubernetes service' do - it 'migrates' do - expect{ migrate! }.to change { Clusters::Cluster.count }.by(1) - - Clusters::Cluster.last.tap do |c| - expect(c).to be_active - expect(c.platform_kubernetes.api_url).to eq(kubernetes_service.api_url) - expect(c.platform_kubernetes.ca_pem).to eq(kubernetes_service.ca_pem) - expect(c.platform_kubernetes.token).to eq(kubernetes_service.token) - end - - expect(kubernetes_service).not_to be_active - end - end - - context 'when user stopped using the kubernetes service' do - before do - kubernetes_service.update(active: false) - end - - it 'migrates' do - expect{ migrate! }.to change { Clusters::Cluster.count }.by(1) - - Clusters::Cluster.last.tap do |c| - expect(c).not_to be_active - expect(c.platform_kubernetes.api_url).to eq(kubernetes_service.api_url) - expect(c.platform_kubernetes.ca_pem).to eq(kubernetes_service.ca_pem) - expect(c.platform_kubernetes.token).to eq(kubernetes_service.token) - end - - expect(kubernetes_service).not_to be_active + it 'migrates the KubernetesService to Platform::Kubernetes' do + expect{ migrate! }.to change { Clusters::Cluster.count }.by(sample_num) + + projects.each do |project| + project.clusters.last.tap do |cluster| + expect(cluster.enabled).to eq(active) + expect(cluster.platform_kubernetes.api_url).to eq(project.kubernetes_service.api_url) + expect(cluster.platform_kubernetes.ca_pem).to eq(project.kubernetes_service.ca_pem) + expect(cluster.platform_kubernetes.token).to eq(project.kubernetes_service.token) + expect(project.kubernetes_service).not_to be_active + expect(project.kubernetes_service.properties['migrated']).to be_truthy end end end end - context 'when user deleted cluster' do - before do - destory_cluster! - end - - context 'when user configured kubernetes from Integration > Kubernetes' do - let!(:new_kubernetes_service) do - project.create_kubernetes_service( - active: true, - api_url: 'http://123.123.123.123', - ca_pem: nil, - token: 'a' * 40) - end - - context 'when user is using the kubernetes service' do - it 'migrates' do - expect{ migrate! }.to change { Clusters::Cluster.count }.by(1) + context 'when KubernetesService is active' do + let(:active) { true } - Clusters::Cluster.last.tap do |c| - expect(c).to be_active - expect(c.platform_kubernetes.api_url).to eq(new_kubernetes_service.api_url) - expect(c.platform_kubernetes.ca_pem).to eq(new_kubernetes_service.ca_pem) - expect(c.platform_kubernetes.token).to eq(new_kubernetes_service.token) - end - - expect(new_kubernetes_service).not_to be_active - end - end - - context 'when user stopped using the kubernetes service' do - before do - new_kubernetes_service.update(active: false) - end + it_behaves_like 'KubernetesService migration' + end - it 'migrates' do - expect{ migrate! }.to change { Clusters::Cluster.count }.by(1) + context 'when KubernetesService is not active' do + let(:active) { false } - Clusters::Cluster.last.tap do |c| - expect(c).not_to be_active - expect(c.platform_kubernetes.api_url).to eq(new_kubernetes_service.api_url) - expect(c.platform_kubernetes.ca_pem).to eq(new_kubernetes_service.ca_pem) - expect(c.platform_kubernetes.token).to eq(new_kubernetes_service.token) - end + # Platforms::Kubernetes validates `token` reagdless of the activeness + # KubernetesService validates `token` if only it's activated + # However, in this migration file, there are no validations because of the migration specific model class + # therefore, Validation Error will not happen in this case and just migrate data + let(:token) { '' } - expect(new_kubernetes_service).not_to be_active - end - end - end + it_behaves_like 'KubernetesService migration' end end - context 'when user configured kubernetes from Integration > Kubernetes' do - let(:project) { create(:project) } - let!(:kubernetes_service) { create(:kubernetes_service, project: project) } - - context 'when user is using the kubernetes service' do - it 'migrates' do - expect{ migrate! }.to change { Clusters::Cluster.count }.by(1) - - Clusters::Cluster.last.tap do |c| - expect(c).to be_active - expect(c.platform_kubernetes.api_url).to eq(kubernetes_service.api_url) - expect(c.platform_kubernetes.ca_pem).to eq(kubernetes_service.ca_pem) - expect(c.platform_kubernetes.token).to eq(kubernetes_service.token) - end + context 'when unique KubernetesService spawned from Service Template' do + it 'migrates the KubernetesService to Platform::Kubernetes' do - expect(kubernetes_service).not_to be_active - end end + end - context 'when user stopped using the kubernetes service' do - before do - kubernetes_service.update(active: false) - end - - it 'migrates' do - expect{ migrate! }.to change { Clusters::Cluster.count }.by(1) - - Clusters::Cluster.last.tap do |c| - expect(c).not_to be_active - expect(c.platform_kubernetes.api_url).to eq(kubernetes_service.api_url) - expect(c.platform_kubernetes.ca_pem).to eq(kubernetes_service.ca_pem) - expect(c.platform_kubernetes.token).to eq(kubernetes_service.token) - end + context 'when synced KubernetesService exists' do + it 'does not migrate the KubernetesService' do # Because the corresponding Platform::Kubernetes already exists - expect(kubernetes_service).not_to be_active - end end end - context 'when nothing is configured' do - it 'migrates' do - expect{ migrate! }.not_to change { Clusters::Cluster.count } - end - end + context 'when KubernetesService does not exist' do + it 'does not migrate the KubernetesService' do - def disable_cluster! - cluster.update!(enabled: false) - kubernetes_service.update!(active: false) + end end - def destory_cluster! - cluster.destroy! - kubernetes_service.destroy! - end + # context 'when user configured kubernetes from CI/CD > Clusters' do + # let(:project) { create(:project) } + # let(:user) { create(:user) } + + # # Platforms::Kubernetes (New archtecture) + # let!(:cluster) do + # create(:cluster, + # projects: [project], + # user: user, + # provider_type: :gcp, + # platform_type: :kubernetes, + # provider_gcp: provider_gcp, + # platform_kubernetes: platform_kubernetes) + # end + + # let(:provider_gcp) { create(:cluster_provider_gcp, :created) } + # let(:platform_kubernetes) { create(:cluster_platform_kubernetes, :configured) } + + # # KubernetesService (Automatically synchronized when Platforms::Kubernetes created) + # let!(:kubernetes_service) { create(:kubernetes_service, project: project) } + + # context 'when user is using the cluster' do + # it 'migrates' do + # expect{ migrate! }.not_to change { Clusters::Cluster.count } + # expect(cluster).to be_active + # expect(kubernetes_service).not_to be_active + # end + # end + + # context 'when user disabled cluster' do + # before do + # disable_cluster! + # end + + # context 'when user configured kubernetes from Integration > Kubernetes' do + # before do + # kubernetes_service.update( + # active: true, + # api_url: 'http://new.kube.com', + # ca_pem: nil, + # token: 'z' * 40).reload + # end + + # context 'when user is using the kubernetes service' do + # it 'migrates' do + # expect{ migrate! }.to change { Clusters::Cluster.count }.by(1) + + # Clusters::Cluster.last.tap do |c| + # expect(c).to be_active + # expect(c.platform_kubernetes.api_url).to eq(kubernetes_service.api_url) + # expect(c.platform_kubernetes.ca_pem).to eq(kubernetes_service.ca_pem) + # expect(c.platform_kubernetes.token).to eq(kubernetes_service.token) + # end + + # expect(kubernetes_service).not_to be_active + # end + # end + + # context 'when user stopped using the kubernetes service' do + # before do + # kubernetes_service.update(active: false) + # end + + # it 'migrates' do + # expect{ migrate! }.to change { Clusters::Cluster.count }.by(1) + + # Clusters::Cluster.last.tap do |c| + # expect(c).not_to be_active + # expect(c.platform_kubernetes.api_url).to eq(kubernetes_service.api_url) + # expect(c.platform_kubernetes.ca_pem).to eq(kubernetes_service.ca_pem) + # expect(c.platform_kubernetes.token).to eq(kubernetes_service.token) + # end + + # expect(kubernetes_service).not_to be_active + # end + # end + # end + # end + + # context 'when user deleted cluster' do + # before do + # destory_cluster! + # end + + # context 'when user configured kubernetes from Integration > Kubernetes' do + # let!(:new_kubernetes_service) do + # project.create_kubernetes_service( + # active: true, + # api_url: 'http://123.123.123.123', + # ca_pem: nil, + # token: 'a' * 40) + # end + + # context 'when user is using the kubernetes service' do + # it 'migrates' do + # expect{ migrate! }.to change { Clusters::Cluster.count }.by(1) + + # Clusters::Cluster.last.tap do |c| + # expect(c).to be_active + # expect(c.platform_kubernetes.api_url).to eq(new_kubernetes_service.api_url) + # expect(c.platform_kubernetes.ca_pem).to eq(new_kubernetes_service.ca_pem) + # expect(c.platform_kubernetes.token).to eq(new_kubernetes_service.token) + # end + + # expect(new_kubernetes_service).not_to be_active + # end + # end + + # context 'when user stopped using the kubernetes service' do + # before do + # new_kubernetes_service.update(active: false) + # end + + # it 'migrates' do + # expect{ migrate! }.to change { Clusters::Cluster.count }.by(1) + + # Clusters::Cluster.last.tap do |c| + # expect(c).not_to be_active + # expect(c.platform_kubernetes.api_url).to eq(new_kubernetes_service.api_url) + # expect(c.platform_kubernetes.ca_pem).to eq(new_kubernetes_service.ca_pem) + # expect(c.platform_kubernetes.token).to eq(new_kubernetes_service.token) + # end + + # expect(new_kubernetes_service).not_to be_active + # end + # end + # end + # end + # end + + # context 'when user configured kubernetes from Integration > Kubernetes' do + # let(:project) { create(:project) } + # let!(:kubernetes_service) { create(:kubernetes_service, project: project) } + + # context 'when user is using the kubernetes service' do + # it 'migrates' do + # expect{ migrate! }.to change { Clusters::Cluster.count }.by(1) + + # Clusters::Cluster.last.tap do |c| + # expect(c).to be_active + # expect(c.platform_kubernetes.api_url).to eq(kubernetes_service.api_url) + # expect(c.platform_kubernetes.ca_pem).to eq(kubernetes_service.ca_pem) + # expect(c.platform_kubernetes.token).to eq(kubernetes_service.token) + # end + + # expect(kubernetes_service).not_to be_active + # end + # end + + # context 'when user stopped using the kubernetes service' do + # before do + # kubernetes_service.update(active: false) + # end + + # it 'migrates' do + # expect{ migrate! }.to change { Clusters::Cluster.count }.by(1) + + # Clusters::Cluster.last.tap do |c| + # expect(c).not_to be_active + # expect(c.platform_kubernetes.api_url).to eq(kubernetes_service.api_url) + # expect(c.platform_kubernetes.ca_pem).to eq(kubernetes_service.ca_pem) + # expect(c.platform_kubernetes.token).to eq(kubernetes_service.token) + # end + + # expect(kubernetes_service).not_to be_active + # end + # end + # end + + # context 'when nothing is configured' do + # it 'migrates' do + # expect{ migrate! }.not_to change { Clusters::Cluster.count } + # end + # end + + # def disable_cluster! + # cluster.update!(enabled: false) + # kubernetes_service.update!(active: false) + # end + + # def destory_cluster! + # cluster.destroy! + # kubernetes_service.destroy! + # end end -- cgit v1.2.1 From 4dc14576d506218c71debb3a0600acdf855afe09 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Fri, 15 Dec 2017 22:41:30 +0900 Subject: Fix comments --- ...bernetes_service_to_new_clusters_architectures.rb | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb b/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb index 7c7e5046ec8..e05d206eb9c 100644 --- a/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb +++ b/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb @@ -52,24 +52,24 @@ class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migrati belongs_to :project, class_name: 'Project' # 10.1 ~ 10.2 - # When users create a cluster, KubernetesService is automatically synchronized - # with Platforms::Kubernetes due to delegate Kubernetes specific logic. - # We only target unmanaged KubernetesService records. + # When users created a cluster, KubernetesService was automatically configured + # by Platforms::Kubernetes parameters. + # Because Platforms::Kubernetes delegated some logic to KubernetesService. # # 10.3 - # We no longer create KubernetesService because Platforms::Kubernetes has the specific logic. + # When users create a cluster, KubernetesService is no longer synchronized. + # Because we copied delegated logic in Platforms::Kubernetes. # - # "unmanaged" means "unmanaged by Platforms::Kubernetes(New archetecture)" - # - # "cluster_projects.project_id IS NULL" -> it's not copied from KubernetesService - # "services.properties NOT LIKE CONCAT('%', cluster_platforms_kubernetes.api_url, '%')" -> KubernetesService has unique configuration which is not included in Platforms::Kubernetes + # NOTE: + # - "unmanaged" means "unmanaged by Platforms::Kubernetes(New archetecture)" + # - We only want to migrate records which are not synchronized with Platforms::Kubernetes. scope :unmanaged_kubernetes_service, -> do - joins('INNER JOIN projects ON projects.id = services.project_id') - .where("services.category = 'deployment'") + where("services.category = 'deployment'") .where("services.type = 'KubernetesService'") .where("services.template = FALSE") .where("NOT EXISTS (?)", PlatformsKubernetes + .joins('INNER JOIN projects ON projects.id = services.project_id') .joins('INNER JOIN cluster_projects ON cluster_projects.project_id = projects.id') .where('cluster_projects.cluster_id = cluster_platforms_kubernetes.cluster_id') .where("services.properties LIKE CONCAT('%', cluster_platforms_kubernetes.api_url, '%')") -- cgit v1.2.1 From 8e6ffe358873ff56eb1d2b6354fac39ce2dfa80e Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Fri, 15 Dec 2017 22:43:25 +0900 Subject: Fix test --- ...s_service_to_new_clusters_architectures_spec.rb | 239 +++++---------------- 1 file changed, 52 insertions(+), 187 deletions(-) diff --git a/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb b/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb index 0f270dd37da..d3fa1fe9054 100644 --- a/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb +++ b/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb @@ -54,205 +54,70 @@ describe MigrateKubernetesServiceToNewClustersArchitectures, :migration do end context 'when unique KubernetesService spawned from Service Template' do - it 'migrates the KubernetesService to Platform::Kubernetes' do + let(:sample_num) { 2 } + let(:projects) { create_list(:project, sample_num) } + + let!(:kubernetes_service_template) do + create(:kubernetes_service, + project: nil, + template: true, + api_url: "https://sample.kubernetes.com", + token: "token-sample", + ca_pem: "ca_pem-sample") + end + + let!(:kubernetes_services) do + projects.map do |project| + create(:kubernetes_service, + project: project, + api_url: kubernetes_service_template.api_url, + token: kubernetes_service_template.token, + ca_pem: kubernetes_service_template.ca_pem) + end + end + + it 'migrates the KubernetesService to Platform::Kubernetes without template' do + expect{ migrate! }.to change { Clusters::Cluster.count }.by(sample_num) + projects.each do |project| + project.clusters.last.tap do |cluster| + expect(cluster.platform_kubernetes.api_url).to eq(project.kubernetes_service.api_url) + expect(cluster.platform_kubernetes.ca_pem).to eq(project.kubernetes_service.ca_pem) + expect(cluster.platform_kubernetes.token).to eq(project.kubernetes_service.token) + expect(project.kubernetes_service).not_to be_active + expect(project.kubernetes_service.properties['migrated']).to be_truthy + end + end end end context 'when synced KubernetesService exists' do + let(:project) { create(:project) } + let(:cluster) { create(:cluster, :provided_by_gcp, projects: [project]) } + let!(:platform_kubernetes) { cluster.platform_kubernetes } + + let!(:kubernetes_service) do + create(:kubernetes_service, + project: project, + active: cluster.enabled, + api_url: platform_kubernetes.api_url, + token: platform_kubernetes.token, + ca_pem: platform_kubernetes.ca_cert) + end + it 'does not migrate the KubernetesService' do # Because the corresponding Platform::Kubernetes already exists + expect{ migrate! }.not_to change { Clusters::Cluster.count } + expect(kubernetes_service).to be_active + expect(kubernetes_service.properties['migrated']).to be_falsy end end context 'when KubernetesService does not exist' do - it 'does not migrate the KubernetesService' do + let!(:project) { create(:project) } + it 'does not migrate the KubernetesService' do + expect{ migrate! }.not_to change { Clusters::Cluster.count } end end - - # context 'when user configured kubernetes from CI/CD > Clusters' do - # let(:project) { create(:project) } - # let(:user) { create(:user) } - - # # Platforms::Kubernetes (New archtecture) - # let!(:cluster) do - # create(:cluster, - # projects: [project], - # user: user, - # provider_type: :gcp, - # platform_type: :kubernetes, - # provider_gcp: provider_gcp, - # platform_kubernetes: platform_kubernetes) - # end - - # let(:provider_gcp) { create(:cluster_provider_gcp, :created) } - # let(:platform_kubernetes) { create(:cluster_platform_kubernetes, :configured) } - - # # KubernetesService (Automatically synchronized when Platforms::Kubernetes created) - # let!(:kubernetes_service) { create(:kubernetes_service, project: project) } - - # context 'when user is using the cluster' do - # it 'migrates' do - # expect{ migrate! }.not_to change { Clusters::Cluster.count } - # expect(cluster).to be_active - # expect(kubernetes_service).not_to be_active - # end - # end - - # context 'when user disabled cluster' do - # before do - # disable_cluster! - # end - - # context 'when user configured kubernetes from Integration > Kubernetes' do - # before do - # kubernetes_service.update( - # active: true, - # api_url: 'http://new.kube.com', - # ca_pem: nil, - # token: 'z' * 40).reload - # end - - # context 'when user is using the kubernetes service' do - # it 'migrates' do - # expect{ migrate! }.to change { Clusters::Cluster.count }.by(1) - - # Clusters::Cluster.last.tap do |c| - # expect(c).to be_active - # expect(c.platform_kubernetes.api_url).to eq(kubernetes_service.api_url) - # expect(c.platform_kubernetes.ca_pem).to eq(kubernetes_service.ca_pem) - # expect(c.platform_kubernetes.token).to eq(kubernetes_service.token) - # end - - # expect(kubernetes_service).not_to be_active - # end - # end - - # context 'when user stopped using the kubernetes service' do - # before do - # kubernetes_service.update(active: false) - # end - - # it 'migrates' do - # expect{ migrate! }.to change { Clusters::Cluster.count }.by(1) - - # Clusters::Cluster.last.tap do |c| - # expect(c).not_to be_active - # expect(c.platform_kubernetes.api_url).to eq(kubernetes_service.api_url) - # expect(c.platform_kubernetes.ca_pem).to eq(kubernetes_service.ca_pem) - # expect(c.platform_kubernetes.token).to eq(kubernetes_service.token) - # end - - # expect(kubernetes_service).not_to be_active - # end - # end - # end - # end - - # context 'when user deleted cluster' do - # before do - # destory_cluster! - # end - - # context 'when user configured kubernetes from Integration > Kubernetes' do - # let!(:new_kubernetes_service) do - # project.create_kubernetes_service( - # active: true, - # api_url: 'http://123.123.123.123', - # ca_pem: nil, - # token: 'a' * 40) - # end - - # context 'when user is using the kubernetes service' do - # it 'migrates' do - # expect{ migrate! }.to change { Clusters::Cluster.count }.by(1) - - # Clusters::Cluster.last.tap do |c| - # expect(c).to be_active - # expect(c.platform_kubernetes.api_url).to eq(new_kubernetes_service.api_url) - # expect(c.platform_kubernetes.ca_pem).to eq(new_kubernetes_service.ca_pem) - # expect(c.platform_kubernetes.token).to eq(new_kubernetes_service.token) - # end - - # expect(new_kubernetes_service).not_to be_active - # end - # end - - # context 'when user stopped using the kubernetes service' do - # before do - # new_kubernetes_service.update(active: false) - # end - - # it 'migrates' do - # expect{ migrate! }.to change { Clusters::Cluster.count }.by(1) - - # Clusters::Cluster.last.tap do |c| - # expect(c).not_to be_active - # expect(c.platform_kubernetes.api_url).to eq(new_kubernetes_service.api_url) - # expect(c.platform_kubernetes.ca_pem).to eq(new_kubernetes_service.ca_pem) - # expect(c.platform_kubernetes.token).to eq(new_kubernetes_service.token) - # end - - # expect(new_kubernetes_service).not_to be_active - # end - # end - # end - # end - # end - - # context 'when user configured kubernetes from Integration > Kubernetes' do - # let(:project) { create(:project) } - # let!(:kubernetes_service) { create(:kubernetes_service, project: project) } - - # context 'when user is using the kubernetes service' do - # it 'migrates' do - # expect{ migrate! }.to change { Clusters::Cluster.count }.by(1) - - # Clusters::Cluster.last.tap do |c| - # expect(c).to be_active - # expect(c.platform_kubernetes.api_url).to eq(kubernetes_service.api_url) - # expect(c.platform_kubernetes.ca_pem).to eq(kubernetes_service.ca_pem) - # expect(c.platform_kubernetes.token).to eq(kubernetes_service.token) - # end - - # expect(kubernetes_service).not_to be_active - # end - # end - - # context 'when user stopped using the kubernetes service' do - # before do - # kubernetes_service.update(active: false) - # end - - # it 'migrates' do - # expect{ migrate! }.to change { Clusters::Cluster.count }.by(1) - - # Clusters::Cluster.last.tap do |c| - # expect(c).not_to be_active - # expect(c.platform_kubernetes.api_url).to eq(kubernetes_service.api_url) - # expect(c.platform_kubernetes.ca_pem).to eq(kubernetes_service.ca_pem) - # expect(c.platform_kubernetes.token).to eq(kubernetes_service.token) - # end - - # expect(kubernetes_service).not_to be_active - # end - # end - # end - - # context 'when nothing is configured' do - # it 'migrates' do - # expect{ migrate! }.not_to change { Clusters::Cluster.count } - # end - # end - - # def disable_cluster! - # cluster.update!(enabled: false) - # kubernetes_service.update!(active: false) - # end - - # def destory_cluster! - # cluster.destroy! - # kubernetes_service.destroy! - # end end -- cgit v1.2.1 From 7eeada80d5c03025c8c37bc1c1567f2500acc8d1 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Mon, 18 Dec 2017 16:32:41 +0900 Subject: Add env_scope tests --- ...rnetes_service_to_new_clusters_architectures.rb | 1 + ...s_service_to_new_clusters_architectures_spec.rb | 32 ++++++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb b/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb index e05d206eb9c..5dee8cb8c5f 100644 --- a/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb +++ b/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb @@ -88,6 +88,7 @@ class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migrati provider_type: Cluster.provider_types[:user], platform_type: Cluster.platform_types[:kubernetes], projects: [kubernetes_service.project], + environment_scope: '*', # KubernetesService is considered as a default cluster platform_kubernetes_attributes: { api_url: kubernetes_service.api_url, ca_cert: kubernetes_service.ca_pem, diff --git a/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb b/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb index d3fa1fe9054..bf41b2df359 100644 --- a/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb +++ b/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb @@ -113,6 +113,38 @@ describe MigrateKubernetesServiceToNewClustersArchitectures, :migration do end end + context 'when production cluster has already been existsed' do + let(:project) { create(:project) } + let!(:cluster) { create(:cluster, :provided_by_gcp, environment_scope: 'production/*', projects: [project]) } + let!(:kubernetes_service) { create(:kubernetes_service, api_url: 'https://debug.kube.com', active: true, project: project) } + + it 'migrates the KubernetesService to Platform::Kubernetes' do + expect{ migrate! }.to change { Clusters::Cluster.count }.by(1) + + kubernetes_service.reload + project.clusters.last.tap do |cluster| + expect(cluster.platform_kubernetes.api_url).to eq(kubernetes_service.api_url) + expect(cluster.platform_kubernetes.ca_pem).to eq(kubernetes_service.ca_pem) + expect(cluster.platform_kubernetes.token).to eq(kubernetes_service.token) + expect(kubernetes_service).not_to be_active + expect(kubernetes_service.properties['migrated']).to be_truthy + end + end + end + + context 'when default cluster has already been existsed' do + let(:project) { create(:project) } + let!(:cluster) { create(:cluster, :provided_by_gcp, environment_scope: '*', projects: [project]) } + let!(:kubernetes_service) { create(:kubernetes_service, api_url: 'https://debug.kube.com', active: true, project: project) } + + it 'does not migrate the KubernetesService' do # Because environment_scope is duplicated + expect{ migrate! }.not_to change { Clusters::Cluster.count } + + expect(kubernetes_service).to be_active + expect(kubernetes_service.properties['migrated']).to be_falsy + end + end + context 'when KubernetesService does not exist' do let!(:project) { create(:project) } -- cgit v1.2.1 From f083739e9935f774984f4c8ba2ce4663c29c7564 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Wed, 3 Jan 2018 20:58:54 +0900 Subject: Add logic to swtich environment_scope by the situation --- ...rnetes_service_to_new_clusters_architectures.rb | 52 ++++++++++++---------- ...s_service_to_new_clusters_architectures_spec.rb | 50 ++++++++++++++++----- 2 files changed, 69 insertions(+), 33 deletions(-) diff --git a/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb b/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb index 5dee8cb8c5f..46b40ea7a44 100644 --- a/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb +++ b/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb @@ -51,36 +51,41 @@ class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migrati belongs_to :project, class_name: 'Project' - # 10.1 ~ 10.2 - # When users created a cluster, KubernetesService was automatically configured - # by Platforms::Kubernetes parameters. - # Because Platforms::Kubernetes delegated some logic to KubernetesService. - # - # 10.3 - # When users create a cluster, KubernetesService is no longer synchronized. - # Because we copied delegated logic in Platforms::Kubernetes. - # - # NOTE: - # - "unmanaged" means "unmanaged by Platforms::Kubernetes(New archetecture)" - # - We only want to migrate records which are not synchronized with Platforms::Kubernetes. - scope :unmanaged_kubernetes_service, -> do + scope :kubernetes_service, -> do where("services.category = 'deployment'") .where("services.type = 'KubernetesService'") .where("services.template = FALSE") - .where("NOT EXISTS (?)", - PlatformsKubernetes - .joins('INNER JOIN projects ON projects.id = services.project_id') - .joins('INNER JOIN cluster_projects ON cluster_projects.project_id = projects.id') - .where('cluster_projects.cluster_id = cluster_platforms_kubernetes.cluster_id') - .where("services.properties LIKE CONCAT('%', cluster_platforms_kubernetes.api_url, '%')") - .select('1') ) .order('services.project_id') end end + def find_dedicated_environement_scope(project) + environment_scopes = project.clusters.map(&:environment_scope) + + return '*' if environment_scopes.exclude?('*') # KubernetesService should be added as a default cluster (environment_scope: '*') at first place + return 'migrated/*' if environment_scopes.exclude?('migrated/*') # If it's conflicted, the KubernetesService added as a migrated cluster + + unique_iid = 0 + + # If it's still conflicted, finding an unique environment scope incrementaly + while true + candidate = "migrated#{unique_iid}/*" + return candidate if environment_scopes.exclude?(candidate) + unique_iid += 1 + end + end + + # KubernetesService might be already managed by clusters + def managed_by_clusters?(kubernetes_service) + kubernetes_service.project.clusters + .joins('INNER JOIN cluster_platforms_kubernetes ON clusters.id = cluster_platforms_kubernetes.cluster_id') + .where('cluster_platforms_kubernetes.api_url = ?', kubernetes_service.api_url) + .exists? + end + def up - Service.unmanaged_kubernetes_service - .find_each(batch_size: 1) do |kubernetes_service| + Service.kubernetes_service.find_each(batch_size: 1) do |kubernetes_service| + unless managed_by_clusters?(kubernetes_service) Cluster.create( enabled: kubernetes_service.active, user_id: nil, # KubernetesService doesn't have @@ -88,7 +93,7 @@ class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migrati provider_type: Cluster.provider_types[:user], platform_type: Cluster.platform_types[:kubernetes], projects: [kubernetes_service.project], - environment_scope: '*', # KubernetesService is considered as a default cluster + environment_scope: find_dedicated_environement_scope(kubernetes_service.project), platform_kubernetes_attributes: { api_url: kubernetes_service.api_url, ca_cert: kubernetes_service.ca_pem, @@ -98,6 +103,7 @@ class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migrati encrypted_password_iv: nil, # KubernetesService doesn't have token: kubernetes_service.token # encrypted_token and encrypted_token_iv } ) + end # Disable the KubernetesService. Platforms::Kubernetes will be used from next time. kubernetes_service.active = false diff --git a/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb b/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb index bf41b2df359..5ce1c43d92a 100644 --- a/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb +++ b/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb @@ -91,7 +91,7 @@ describe MigrateKubernetesServiceToNewClustersArchitectures, :migration do end end - context 'when synced KubernetesService exists' do + context 'when managed KubernetesService exists' do let(:project) { create(:project) } let(:cluster) { create(:cluster, :provided_by_gcp, projects: [project]) } let!(:platform_kubernetes) { cluster.platform_kubernetes } @@ -105,15 +105,16 @@ describe MigrateKubernetesServiceToNewClustersArchitectures, :migration do ca_pem: platform_kubernetes.ca_cert) end - it 'does not migrate the KubernetesService' do # Because the corresponding Platform::Kubernetes already exists + it 'does not migrate the KubernetesService and disables the kubernetes_service' do # Because the corresponding Platform::Kubernetes already exists expect{ migrate! }.not_to change { Clusters::Cluster.count } - expect(kubernetes_service).to be_active - expect(kubernetes_service.properties['migrated']).to be_falsy + kubernetes_service.reload + expect(kubernetes_service).not_to be_active + expect(kubernetes_service.properties['migrated']).to be_truthy end end - context 'when production cluster has already been existsed' do + context 'when production cluster has already been existed' do # i.e. There are no environment_scope conflicts let(:project) { create(:project) } let!(:cluster) { create(:cluster, :provided_by_gcp, environment_scope: 'production/*', projects: [project]) } let!(:kubernetes_service) { create(:kubernetes_service, api_url: 'https://debug.kube.com', active: true, project: project) } @@ -123,6 +124,7 @@ describe MigrateKubernetesServiceToNewClustersArchitectures, :migration do kubernetes_service.reload project.clusters.last.tap do |cluster| + expect(cluster.environment_scope).to eq('*') expect(cluster.platform_kubernetes.api_url).to eq(kubernetes_service.api_url) expect(cluster.platform_kubernetes.ca_pem).to eq(kubernetes_service.ca_pem) expect(cluster.platform_kubernetes.token).to eq(kubernetes_service.token) @@ -132,16 +134,44 @@ describe MigrateKubernetesServiceToNewClustersArchitectures, :migration do end end - context 'when default cluster has already been existsed' do + context 'when default cluster has already been existed' do let(:project) { create(:project) } let!(:cluster) { create(:cluster, :provided_by_gcp, environment_scope: '*', projects: [project]) } let!(:kubernetes_service) { create(:kubernetes_service, api_url: 'https://debug.kube.com', active: true, project: project) } - it 'does not migrate the KubernetesService' do # Because environment_scope is duplicated - expect{ migrate! }.not_to change { Clusters::Cluster.count } + it 'migrates the KubernetesService to Platform::Kubernetes with dedicated environment_scope' do # Because environment_scope is duplicated + expect{ migrate! }.to change { Clusters::Cluster.count }.by(1) + + kubernetes_service.reload + project.clusters.last.tap do |cluster| + expect(cluster.environment_scope).to eq('migrated/*') + expect(cluster.platform_kubernetes.api_url).to eq(kubernetes_service.api_url) + expect(cluster.platform_kubernetes.ca_pem).to eq(kubernetes_service.ca_pem) + expect(cluster.platform_kubernetes.token).to eq(kubernetes_service.token) + expect(kubernetes_service).not_to be_active + expect(kubernetes_service.properties['migrated']).to be_truthy + end + end + end - expect(kubernetes_service).to be_active - expect(kubernetes_service.properties['migrated']).to be_falsy + context 'when default cluster and migrated cluster has already been existed' do + let(:project) { create(:project) } + let!(:cluster) { create(:cluster, :provided_by_gcp, environment_scope: '*', projects: [project]) } + let!(:migrated_cluster) { create(:cluster, :provided_by_gcp, environment_scope: 'migrated/*', projects: [project]) } + let!(:kubernetes_service) { create(:kubernetes_service, api_url: 'https://debug.kube.com', active: true, project: project) } + + it 'migrates the KubernetesService to Platform::Kubernetes with dedicated environment_scope' do # Because environment_scope is duplicated + expect{ migrate! }.to change { Clusters::Cluster.count }.by(1) + + kubernetes_service.reload + project.clusters.last.tap do |cluster| + expect(cluster.environment_scope).to eq('migrated0/*') + expect(cluster.platform_kubernetes.api_url).to eq(kubernetes_service.api_url) + expect(cluster.platform_kubernetes.ca_pem).to eq(kubernetes_service.ca_pem) + expect(cluster.platform_kubernetes.token).to eq(kubernetes_service.token) + expect(kubernetes_service).not_to be_active + expect(kubernetes_service.properties['migrated']).to be_truthy + end end end -- cgit v1.2.1 From 9b7719b6c60f2ba9a2e8e6ded543bb4fb87cd11b Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Wed, 3 Jan 2018 23:44:08 +0900 Subject: Use explicit namespace for avoiding reference from application code --- ...rnetes_service_to_new_clusters_architectures.rb | 29 ++++++++++++---------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb b/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb index 46b40ea7a44..da34880b4e1 100644 --- a/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb +++ b/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb @@ -5,16 +5,17 @@ class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migrati class Project < ActiveRecord::Base self.table_name = 'projects' - has_many :cluster_projects, class_name: 'ClustersProject' - has_many :clusters, through: :cluster_projects, class_name: 'Cluster' + has_many :cluster_projects, class_name: 'MigrateKubernetesServiceToNewClustersArchitectures::ClustersProject' + has_many :clusters, through: :cluster_projects, class_name: 'MigrateKubernetesServiceToNewClustersArchitectures::Cluster' + has_many :services, class_name: 'MigrateKubernetesServiceToNewClustersArchitectures::Service' end class Cluster < ActiveRecord::Base self.table_name = 'clusters' - has_many :cluster_projects, class_name: 'ClustersProject' - has_many :projects, through: :cluster_projects, class_name: 'Project' - has_one :platform_kubernetes, class_name: 'PlatformsKubernetes' + has_many :cluster_projects, class_name: 'MigrateKubernetesServiceToNewClustersArchitectures::ClustersProject' + has_many :projects, through: :cluster_projects, class_name: 'MigrateKubernetesServiceToNewClustersArchitectures::Project' + has_one :platform_kubernetes, class_name: 'MigrateKubernetesServiceToNewClustersArchitectures::PlatformsKubernetes' accepts_nested_attributes_for :platform_kubernetes @@ -31,13 +32,15 @@ class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migrati class ClustersProject < ActiveRecord::Base self.table_name = 'cluster_projects' - belongs_to :cluster, class_name: 'Cluster' - belongs_to :project, class_name: 'Project' + belongs_to :cluster, class_name: 'MigrateKubernetesServiceToNewClustersArchitectures::Cluster' + belongs_to :project, class_name: 'MigrateKubernetesServiceToNewClustersArchitectures::Project' end class PlatformsKubernetes < ActiveRecord::Base self.table_name = 'cluster_platforms_kubernetes' + belongs_to :cluster, class_name: 'MigrateKubernetesServiceToNewClustersArchitectures::Cluster' + attr_encrypted :token, mode: :per_attribute_iv, key: Gitlab::Application.secrets.db_key_base, @@ -49,7 +52,7 @@ class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migrati self.table_name = 'services' - belongs_to :project, class_name: 'Project' + belongs_to :project, class_name: 'MigrateKubernetesServiceToNewClustersArchitectures::Project' scope :kubernetes_service, -> do where("services.category = 'deployment'") @@ -84,15 +87,15 @@ class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migrati end def up - Service.kubernetes_service.find_each(batch_size: 1) do |kubernetes_service| + MigrateKubernetesServiceToNewClustersArchitectures::Service.kubernetes_service.find_each(batch_size: 1) do |kubernetes_service| unless managed_by_clusters?(kubernetes_service) - Cluster.create( + MigrateKubernetesServiceToNewClustersArchitectures::Cluster.create( enabled: kubernetes_service.active, user_id: nil, # KubernetesService doesn't have name: DEFAULT_KUBERNETES_SERVICE_CLUSTER_NAME, - provider_type: Cluster.provider_types[:user], - platform_type: Cluster.platform_types[:kubernetes], - projects: [kubernetes_service.project], + provider_type: MigrateKubernetesServiceToNewClustersArchitectures::Cluster.provider_types[:user], + platform_type: MigrateKubernetesServiceToNewClustersArchitectures::Cluster.platform_types[:kubernetes], + projects: [kubernetes_service.project.becomes(MigrateKubernetesServiceToNewClustersArchitectures::Project)], environment_scope: find_dedicated_environement_scope(kubernetes_service.project), platform_kubernetes_attributes: { api_url: kubernetes_service.api_url, -- cgit v1.2.1 From 665972e2f51ae8a72a307dc4926d379bff816336 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Wed, 3 Jan 2018 23:49:34 +0900 Subject: Avoid quotes in ActiveRecord query --- ...27_migrate_kubernetes_service_to_new_clusters_architectures.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb b/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb index da34880b4e1..e6c7ea58bd2 100644 --- a/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb +++ b/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb @@ -55,10 +55,10 @@ class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migrati belongs_to :project, class_name: 'MigrateKubernetesServiceToNewClustersArchitectures::Project' scope :kubernetes_service, -> do - where("services.category = 'deployment'") - .where("services.type = 'KubernetesService'") - .where("services.template = FALSE") - .order('services.project_id') + where(category: 'deployment') + .where(type: 'KubernetesService') + .where(template: false) + .order(project_id: :asc) end end -- cgit v1.2.1 From c80598816c4cb79c25255ecd53906c84fc215a87 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Thu, 4 Jan 2018 23:49:42 +0900 Subject: Opitmize migration process by using both unmanaged_kubernetes_service and kubernetes_service_without_template --- ...rnetes_service_to_new_clusters_architectures.rb | 70 +++++++++++----------- ...s_service_to_new_clusters_architectures_spec.rb | 6 -- 2 files changed, 36 insertions(+), 40 deletions(-) diff --git a/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb b/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb index e6c7ea58bd2..32f8ef3ff02 100644 --- a/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb +++ b/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb @@ -54,7 +54,21 @@ class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migrati belongs_to :project, class_name: 'MigrateKubernetesServiceToNewClustersArchitectures::Project' - scope :kubernetes_service, -> do + scope :unmanaged_kubernetes_service, -> do + where(category: 'deployment') + .where(type: 'KubernetesService') + .where(template: false) + .where("NOT EXISTS (?)", + MigrateKubernetesServiceToNewClustersArchitectures::PlatformsKubernetes + .joins('INNER JOIN projects ON projects.id = services.project_id') + .joins('INNER JOIN cluster_projects ON cluster_projects.project_id = projects.id') + .where('cluster_projects.cluster_id = cluster_platforms_kubernetes.cluster_id') + .where("services.properties LIKE CONCAT('%', cluster_platforms_kubernetes.api_url, '%')") + .select('1') ) + .order(project_id: :asc) + end + + scope :kubernetes_service_without_template, -> do where(category: 'deployment') .where(type: 'KubernetesService') .where(template: false) @@ -78,41 +92,29 @@ class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migrati end end - # KubernetesService might be already managed by clusters - def managed_by_clusters?(kubernetes_service) - kubernetes_service.project.clusters - .joins('INNER JOIN cluster_platforms_kubernetes ON clusters.id = cluster_platforms_kubernetes.cluster_id') - .where('cluster_platforms_kubernetes.api_url = ?', kubernetes_service.api_url) - .exists? - end - def up - MigrateKubernetesServiceToNewClustersArchitectures::Service.kubernetes_service.find_each(batch_size: 1) do |kubernetes_service| - unless managed_by_clusters?(kubernetes_service) - MigrateKubernetesServiceToNewClustersArchitectures::Cluster.create( - enabled: kubernetes_service.active, - user_id: nil, # KubernetesService doesn't have - name: DEFAULT_KUBERNETES_SERVICE_CLUSTER_NAME, - provider_type: MigrateKubernetesServiceToNewClustersArchitectures::Cluster.provider_types[:user], - platform_type: MigrateKubernetesServiceToNewClustersArchitectures::Cluster.platform_types[:kubernetes], - projects: [kubernetes_service.project.becomes(MigrateKubernetesServiceToNewClustersArchitectures::Project)], - environment_scope: find_dedicated_environement_scope(kubernetes_service.project), - platform_kubernetes_attributes: { - api_url: kubernetes_service.api_url, - ca_cert: kubernetes_service.ca_pem, - namespace: kubernetes_service.namespace, - username: nil, # KubernetesService doesn't have - encrypted_password: nil, # KubernetesService doesn't have - encrypted_password_iv: nil, # KubernetesService doesn't have - token: kubernetes_service.token # encrypted_token and encrypted_token_iv - } ) - end - - # Disable the KubernetesService. Platforms::Kubernetes will be used from next time. - kubernetes_service.active = false - kubernetes_service.properties.merge!( { migrated: true } ) - kubernetes_service.save! + MigrateKubernetesServiceToNewClustersArchitectures::Service + .unmanaged_kubernetes_service.find_each(batch_size: 1) do |kubernetes_service| + MigrateKubernetesServiceToNewClustersArchitectures::Cluster.create( + enabled: kubernetes_service.active, + user_id: nil, # KubernetesService doesn't have + name: DEFAULT_KUBERNETES_SERVICE_CLUSTER_NAME, + provider_type: MigrateKubernetesServiceToNewClustersArchitectures::Cluster.provider_types[:user], + platform_type: MigrateKubernetesServiceToNewClustersArchitectures::Cluster.platform_types[:kubernetes], + projects: [kubernetes_service.project.becomes(MigrateKubernetesServiceToNewClustersArchitectures::Project)], + environment_scope: find_dedicated_environement_scope(kubernetes_service.project), + platform_kubernetes_attributes: { + api_url: kubernetes_service.api_url, + ca_cert: kubernetes_service.ca_pem, + namespace: kubernetes_service.namespace, + username: nil, # KubernetesService doesn't have + encrypted_password: nil, # KubernetesService doesn't have + encrypted_password_iv: nil, # KubernetesService doesn't have + token: kubernetes_service.token # encrypted_token and encrypted_token_iv + } ) end + + MigrateKubernetesServiceToNewClustersArchitectures::Service.kubernetes_service_without_template.update_all(active: false) end def down diff --git a/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb b/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb index 5ce1c43d92a..d6643a63f0d 100644 --- a/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb +++ b/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb @@ -28,7 +28,6 @@ describe MigrateKubernetesServiceToNewClustersArchitectures, :migration do expect(cluster.platform_kubernetes.ca_pem).to eq(project.kubernetes_service.ca_pem) expect(cluster.platform_kubernetes.token).to eq(project.kubernetes_service.token) expect(project.kubernetes_service).not_to be_active - expect(project.kubernetes_service.properties['migrated']).to be_truthy end end end @@ -85,7 +84,6 @@ describe MigrateKubernetesServiceToNewClustersArchitectures, :migration do expect(cluster.platform_kubernetes.ca_pem).to eq(project.kubernetes_service.ca_pem) expect(cluster.platform_kubernetes.token).to eq(project.kubernetes_service.token) expect(project.kubernetes_service).not_to be_active - expect(project.kubernetes_service.properties['migrated']).to be_truthy end end end @@ -110,7 +108,6 @@ describe MigrateKubernetesServiceToNewClustersArchitectures, :migration do kubernetes_service.reload expect(kubernetes_service).not_to be_active - expect(kubernetes_service.properties['migrated']).to be_truthy end end @@ -129,7 +126,6 @@ describe MigrateKubernetesServiceToNewClustersArchitectures, :migration do expect(cluster.platform_kubernetes.ca_pem).to eq(kubernetes_service.ca_pem) expect(cluster.platform_kubernetes.token).to eq(kubernetes_service.token) expect(kubernetes_service).not_to be_active - expect(kubernetes_service.properties['migrated']).to be_truthy end end end @@ -149,7 +145,6 @@ describe MigrateKubernetesServiceToNewClustersArchitectures, :migration do expect(cluster.platform_kubernetes.ca_pem).to eq(kubernetes_service.ca_pem) expect(cluster.platform_kubernetes.token).to eq(kubernetes_service.token) expect(kubernetes_service).not_to be_active - expect(kubernetes_service.properties['migrated']).to be_truthy end end end @@ -170,7 +165,6 @@ describe MigrateKubernetesServiceToNewClustersArchitectures, :migration do expect(cluster.platform_kubernetes.ca_pem).to eq(kubernetes_service.ca_pem) expect(cluster.platform_kubernetes.token).to eq(kubernetes_service.token) expect(kubernetes_service).not_to be_active - expect(kubernetes_service.properties['migrated']).to be_truthy end end end -- cgit v1.2.1 From b8a275d3e46a4204505ed5a4b7a9b3a6d49c9b4f Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Fri, 5 Jan 2018 00:53:50 +0900 Subject: Use bulk_insert instead of AR create --- ...rnetes_service_to_new_clusters_architectures.rb | 57 +++++++++++++++++----- 1 file changed, 45 insertions(+), 12 deletions(-) diff --git a/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb b/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb index 32f8ef3ff02..f2455a3d8b0 100644 --- a/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb +++ b/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb @@ -94,24 +94,57 @@ class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migrati def up MigrateKubernetesServiceToNewClustersArchitectures::Service - .unmanaged_kubernetes_service.find_each(batch_size: 1) do |kubernetes_service| - MigrateKubernetesServiceToNewClustersArchitectures::Cluster.create( - enabled: kubernetes_service.active, - user_id: nil, # KubernetesService doesn't have - name: DEFAULT_KUBERNETES_SERVICE_CLUSTER_NAME, - provider_type: MigrateKubernetesServiceToNewClustersArchitectures::Cluster.provider_types[:user], - platform_type: MigrateKubernetesServiceToNewClustersArchitectures::Cluster.platform_types[:kubernetes], - projects: [kubernetes_service.project.becomes(MigrateKubernetesServiceToNewClustersArchitectures::Project)], - environment_scope: find_dedicated_environement_scope(kubernetes_service.project), - platform_kubernetes_attributes: { + .unmanaged_kubernetes_service.each_batch(of: 100) do |kubernetes_services| + + rows_for_clusters = kubernetes_services.map do |kubernetes_service| + { + enabled: kubernetes_service.active, + user_id: nil, # KubernetesService doesn't have + name: DEFAULT_KUBERNETES_SERVICE_CLUSTER_NAME, + provider_type: MigrateKubernetesServiceToNewClustersArchitectures::Cluster.provider_types[:user], + platform_type: MigrateKubernetesServiceToNewClustersArchitectures::Cluster.platform_types[:kubernetes], + environment_scope: find_dedicated_environement_scope(kubernetes_service.project), + created_at: Gitlab::Database.sanitize_timestamp(kubernetes_service.created_at), + updated_at: Gitlab::Database.sanitize_timestamp(kubernetes_service.updated_at) + } + end + + inserted_cluster_ids = Gitlab::Database.bulk_insert('clusters', rows_for_clusters, return_ids: true) + + rows_for_cluster_platforms_kubernetes = kubernetes_services.each_with_index.map do |kubernetes_service, i| + + # Create PlatformsKubernetes instance for generating an encrypted token + platforms_kubernetes = + MigrateKubernetesServiceToNewClustersArchitectures::PlatformsKubernetes + .new(token: kubernetes_service.token) + + { + cluster_id: inserted_cluster_ids[i], api_url: kubernetes_service.api_url, ca_cert: kubernetes_service.ca_pem, namespace: kubernetes_service.namespace, username: nil, # KubernetesService doesn't have encrypted_password: nil, # KubernetesService doesn't have encrypted_password_iv: nil, # KubernetesService doesn't have - token: kubernetes_service.token # encrypted_token and encrypted_token_iv - } ) + encrypted_token: platforms_kubernetes.encrypted_token, # encrypted_token and encrypted_token_iv + encrypted_token_iv: platforms_kubernetes.encrypted_token_iv, # encrypted_token and encrypted_token_iv + created_at: Gitlab::Database.sanitize_timestamp(kubernetes_service.created_at), + updated_at: Gitlab::Database.sanitize_timestamp(kubernetes_service.updated_at) + } + end + + Gitlab::Database.bulk_insert('cluster_platforms_kubernetes', rows_for_cluster_platforms_kubernetes) + + rows_for_cluster_projects = kubernetes_services.each_with_index.map do |kubernetes_service, i| + { + cluster_id: inserted_cluster_ids[i], + project_id: kubernetes_service.project_id, + created_at: Gitlab::Database.sanitize_timestamp(kubernetes_service.created_at), + updated_at: Gitlab::Database.sanitize_timestamp(kubernetes_service.updated_at) + } + end + + Gitlab::Database.bulk_insert('cluster_projects', rows_for_cluster_projects) end MigrateKubernetesServiceToNewClustersArchitectures::Service.kubernetes_service_without_template.update_all(active: false) -- cgit v1.2.1 From acfb8464bef7b43b730ddf433a8223fc240d968a Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Fri, 5 Jan 2018 15:22:45 +0900 Subject: Fix static anylysy --- ...ate_kubernetes_service_to_new_clusters_architectures.rb | 5 +++-- ...ubernetes_service_to_new_clusters_architectures_spec.rb | 14 +++++++------- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb b/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb index f2455a3d8b0..5ea115cea90 100644 --- a/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb +++ b/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb @@ -66,7 +66,7 @@ class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migrati .where("services.properties LIKE CONCAT('%', cluster_platforms_kubernetes.api_url, '%')") .select('1') ) .order(project_id: :asc) - end + end scope :kubernetes_service_without_template, -> do where(category: 'deployment') @@ -85,9 +85,10 @@ class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migrati unique_iid = 0 # If it's still conflicted, finding an unique environment scope incrementaly - while true + loop do candidate = "migrated#{unique_iid}/*" return candidate if environment_scopes.exclude?(candidate) + unique_iid += 1 end end diff --git a/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb b/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb index d6643a63f0d..cade4d7df28 100644 --- a/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb +++ b/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb @@ -19,7 +19,7 @@ describe MigrateKubernetesServiceToNewClustersArchitectures, :migration do end it 'migrates the KubernetesService to Platform::Kubernetes' do - expect{ migrate! }.to change { Clusters::Cluster.count }.by(sample_num) + expect { migrate! }.to change { Clusters::Cluster.count }.by(sample_num) projects.each do |project| project.clusters.last.tap do |cluster| @@ -76,7 +76,7 @@ describe MigrateKubernetesServiceToNewClustersArchitectures, :migration do end it 'migrates the KubernetesService to Platform::Kubernetes without template' do - expect{ migrate! }.to change { Clusters::Cluster.count }.by(sample_num) + expect { migrate! }.to change { Clusters::Cluster.count }.by(sample_num) projects.each do |project| project.clusters.last.tap do |cluster| @@ -104,7 +104,7 @@ describe MigrateKubernetesServiceToNewClustersArchitectures, :migration do end it 'does not migrate the KubernetesService and disables the kubernetes_service' do # Because the corresponding Platform::Kubernetes already exists - expect{ migrate! }.not_to change { Clusters::Cluster.count } + expect { migrate! }.not_to change { Clusters::Cluster.count } kubernetes_service.reload expect(kubernetes_service).not_to be_active @@ -117,7 +117,7 @@ describe MigrateKubernetesServiceToNewClustersArchitectures, :migration do let!(:kubernetes_service) { create(:kubernetes_service, api_url: 'https://debug.kube.com', active: true, project: project) } it 'migrates the KubernetesService to Platform::Kubernetes' do - expect{ migrate! }.to change { Clusters::Cluster.count }.by(1) + expect { migrate! }.to change { Clusters::Cluster.count }.by(1) kubernetes_service.reload project.clusters.last.tap do |cluster| @@ -136,7 +136,7 @@ describe MigrateKubernetesServiceToNewClustersArchitectures, :migration do let!(:kubernetes_service) { create(:kubernetes_service, api_url: 'https://debug.kube.com', active: true, project: project) } it 'migrates the KubernetesService to Platform::Kubernetes with dedicated environment_scope' do # Because environment_scope is duplicated - expect{ migrate! }.to change { Clusters::Cluster.count }.by(1) + expect { migrate! }.to change { Clusters::Cluster.count }.by(1) kubernetes_service.reload project.clusters.last.tap do |cluster| @@ -156,7 +156,7 @@ describe MigrateKubernetesServiceToNewClustersArchitectures, :migration do let!(:kubernetes_service) { create(:kubernetes_service, api_url: 'https://debug.kube.com', active: true, project: project) } it 'migrates the KubernetesService to Platform::Kubernetes with dedicated environment_scope' do # Because environment_scope is duplicated - expect{ migrate! }.to change { Clusters::Cluster.count }.by(1) + expect { migrate! }.to change { Clusters::Cluster.count }.by(1) kubernetes_service.reload project.clusters.last.tap do |cluster| @@ -173,7 +173,7 @@ describe MigrateKubernetesServiceToNewClustersArchitectures, :migration do let!(:project) { create(:project) } it 'does not migrate the KubernetesService' do - expect{ migrate! }.not_to change { Clusters::Cluster.count } + expect { migrate! }.not_to change { Clusters::Cluster.count } end end end -- cgit v1.2.1 From 8bc3221f2fe096e6c1f2070a2fcdb18903c2c599 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Fri, 5 Jan 2018 19:43:03 +0900 Subject: Fix query to look for proper unmanaged kubernetes service --- ...rnetes_service_to_new_clusters_architectures.rb | 29 +++++++++------------- 1 file changed, 12 insertions(+), 17 deletions(-) diff --git a/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb b/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb index 5ea115cea90..aff934c2ab4 100644 --- a/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb +++ b/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb @@ -55,24 +55,16 @@ class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migrati belongs_to :project, class_name: 'MigrateKubernetesServiceToNewClustersArchitectures::Project' scope :unmanaged_kubernetes_service, -> do - where(category: 'deployment') + joins('LEFT JOIN projects ON projects.id = services.project_id') + .joins('LEFT JOIN cluster_projects ON cluster_projects.project_id = projects.id') + .joins('LEFT JOIN cluster_platforms_kubernetes ON cluster_platforms_kubernetes.cluster_id = cluster_projects.cluster_id') + .where(category: 'deployment') .where(type: 'KubernetesService') .where(template: false) - .where("NOT EXISTS (?)", - MigrateKubernetesServiceToNewClustersArchitectures::PlatformsKubernetes - .joins('INNER JOIN projects ON projects.id = services.project_id') - .joins('INNER JOIN cluster_projects ON cluster_projects.project_id = projects.id') - .where('cluster_projects.cluster_id = cluster_platforms_kubernetes.cluster_id') - .where("services.properties LIKE CONCAT('%', cluster_platforms_kubernetes.api_url, '%')") - .select('1') ) - .order(project_id: :asc) - end - - scope :kubernetes_service_without_template, -> do - where(category: 'deployment') - .where(type: 'KubernetesService') - .where(template: false) - .order(project_id: :asc) + .where("services.properties LIKE '%api_url%'") + .where("(services.properties NOT LIKE CONCAT('%', cluster_platforms_kubernetes.api_url, '%')) OR cluster_platforms_kubernetes.api_url IS NULL") + .group(:id) + .order(id: :asc) end end @@ -148,7 +140,10 @@ class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migrati Gitlab::Database.bulk_insert('cluster_projects', rows_for_cluster_projects) end - MigrateKubernetesServiceToNewClustersArchitectures::Service.kubernetes_service_without_template.update_all(active: false) + connection.execute <<~SQL + UPDATE services SET active = false + WHERE category = 'deployment' AND type = 'KubernetesService' AND template = false + SQL end def down -- cgit v1.2.1 From 2d3c7d29b2950110f51cde9b6c8c39f9d7404884 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Fri, 5 Jan 2018 23:24:51 +0900 Subject: Use batch update for Service deactivation --- ...7_migrate_kubernetes_service_to_new_clusters_architectures.rb | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb b/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb index aff934c2ab4..2808c8e0222 100644 --- a/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb +++ b/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb @@ -140,10 +140,11 @@ class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migrati Gitlab::Database.bulk_insert('cluster_projects', rows_for_cluster_projects) end - connection.execute <<~SQL - UPDATE services SET active = false - WHERE category = 'deployment' AND type = 'KubernetesService' AND template = false - SQL + MigrateKubernetesServiceToNewClustersArchitectures::Service + .where(category: 'deployment', type: 'KubernetesService', template: false) + .each_batch(of: 100) do |batch| + batch.update_all(active: false) + end end def down -- cgit v1.2.1 From 1c404c91b6e0bb0fac335083eff2f286d0da6df1 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Sat, 6 Jan 2018 00:37:40 +0900 Subject: Add a new test for emptified params --- ...es_service_to_new_clusters_architectures_spec.rb | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb b/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb index cade4d7df28..5f135ad274e 100644 --- a/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb +++ b/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb @@ -42,8 +42,8 @@ describe MigrateKubernetesServiceToNewClustersArchitectures, :migration do context 'when KubernetesService is not active' do let(:active) { false } - # Platforms::Kubernetes validates `token` reagdless of the activeness - # KubernetesService validates `token` if only it's activated + # Platforms::Kubernetes validates `token` reagdless of the activeness, + # whereas KubernetesService validates `token` if only it's activated # However, in this migration file, there are no validations because of the migration specific model class # therefore, Validation Error will not happen in this case and just migrate data let(:token) { '' } @@ -169,6 +169,23 @@ describe MigrateKubernetesServiceToNewClustersArchitectures, :migration do end end + context 'when KubernetesService has nullified parameters' do + let(:project) { create(:project) } + + before do + ActiveRecord::Base.connection.execute <<~SQL + INSERT INTO services (project_id, active, category, type, properties) + VALUES (#{project.id}, false, 'deployment', 'KubernetesService', '{}'); + SQL + end + + it 'does not migrate the KubernetesService and disables the kubernetes_service' do + expect { migrate! }.not_to change { Clusters::Cluster.count } + + expect(project.kubernetes_service).not_to be_active + end + end + context 'when KubernetesService does not exist' do let!(:project) { create(:project) } -- cgit v1.2.1 From 183dbdc8b8a68bdcbfb465abcbdae2d2292d9386 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Sat, 6 Jan 2018 01:11:06 +0900 Subject: Revert bulk_insert and bring back AR insert(one by one) --- ...rnetes_service_to_new_clusters_architectures.rb | 78 ++++++++-------------- ...s_service_to_new_clusters_architectures_spec.rb | 40 +++++++---- 2 files changed, 54 insertions(+), 64 deletions(-) diff --git a/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb b/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb index 2808c8e0222..27d56fe5b42 100644 --- a/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb +++ b/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb @@ -1,7 +1,11 @@ class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + DOWNTIME = false DEFAULT_KUBERNETES_SERVICE_CLUSTER_NAME = 'KubernetesService'.freeze + disable_ddl_transaction! + class Project < ActiveRecord::Base self.table_name = 'projects' @@ -58,14 +62,16 @@ class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migrati joins('LEFT JOIN projects ON projects.id = services.project_id') .joins('LEFT JOIN cluster_projects ON cluster_projects.project_id = projects.id') .joins('LEFT JOIN cluster_platforms_kubernetes ON cluster_platforms_kubernetes.cluster_id = cluster_projects.cluster_id') - .where(category: 'deployment') - .where(type: 'KubernetesService') - .where(template: false) + .where(category: 'deployment', type: 'KubernetesService', template: false) .where("services.properties LIKE '%api_url%'") .where("(services.properties NOT LIKE CONCAT('%', cluster_platforms_kubernetes.api_url, '%')) OR cluster_platforms_kubernetes.api_url IS NULL") .group(:id) .order(id: :asc) end + + scope :kubernetes_service_without_template, -> do + where(category: 'deployment', type: 'KubernetesService', template: false) + end end def find_dedicated_environement_scope(project) @@ -86,65 +92,33 @@ class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migrati end def up - MigrateKubernetesServiceToNewClustersArchitectures::Service - .unmanaged_kubernetes_service.each_batch(of: 100) do |kubernetes_services| - - rows_for_clusters = kubernetes_services.map do |kubernetes_service| - { + ActiveRecord::Base.transaction do + MigrateKubernetesServiceToNewClustersArchitectures::Service + .unmanaged_kubernetes_service.find_each(batch_size: 1) do |kubernetes_service| + MigrateKubernetesServiceToNewClustersArchitectures::Cluster.create( enabled: kubernetes_service.active, user_id: nil, # KubernetesService doesn't have name: DEFAULT_KUBERNETES_SERVICE_CLUSTER_NAME, provider_type: MigrateKubernetesServiceToNewClustersArchitectures::Cluster.provider_types[:user], platform_type: MigrateKubernetesServiceToNewClustersArchitectures::Cluster.platform_types[:kubernetes], + projects: [kubernetes_service.project.becomes(MigrateKubernetesServiceToNewClustersArchitectures::Project)], environment_scope: find_dedicated_environement_scope(kubernetes_service.project), - created_at: Gitlab::Database.sanitize_timestamp(kubernetes_service.created_at), - updated_at: Gitlab::Database.sanitize_timestamp(kubernetes_service.updated_at) - } - end - - inserted_cluster_ids = Gitlab::Database.bulk_insert('clusters', rows_for_clusters, return_ids: true) - - rows_for_cluster_platforms_kubernetes = kubernetes_services.each_with_index.map do |kubernetes_service, i| - - # Create PlatformsKubernetes instance for generating an encrypted token - platforms_kubernetes = - MigrateKubernetesServiceToNewClustersArchitectures::PlatformsKubernetes - .new(token: kubernetes_service.token) - - { - cluster_id: inserted_cluster_ids[i], - api_url: kubernetes_service.api_url, - ca_cert: kubernetes_service.ca_pem, - namespace: kubernetes_service.namespace, - username: nil, # KubernetesService doesn't have - encrypted_password: nil, # KubernetesService doesn't have - encrypted_password_iv: nil, # KubernetesService doesn't have - encrypted_token: platforms_kubernetes.encrypted_token, # encrypted_token and encrypted_token_iv - encrypted_token_iv: platforms_kubernetes.encrypted_token_iv, # encrypted_token and encrypted_token_iv - created_at: Gitlab::Database.sanitize_timestamp(kubernetes_service.created_at), - updated_at: Gitlab::Database.sanitize_timestamp(kubernetes_service.updated_at) - } - end - - Gitlab::Database.bulk_insert('cluster_platforms_kubernetes', rows_for_cluster_platforms_kubernetes) - - rows_for_cluster_projects = kubernetes_services.each_with_index.map do |kubernetes_service, i| - { - cluster_id: inserted_cluster_ids[i], - project_id: kubernetes_service.project_id, - created_at: Gitlab::Database.sanitize_timestamp(kubernetes_service.created_at), - updated_at: Gitlab::Database.sanitize_timestamp(kubernetes_service.updated_at) - } + platform_kubernetes_attributes: { + api_url: kubernetes_service.api_url, + ca_cert: kubernetes_service.ca_pem, + namespace: kubernetes_service.namespace, + username: nil, # KubernetesService doesn't have + encrypted_password: nil, # KubernetesService doesn't have + encrypted_password_iv: nil, # KubernetesService doesn't have + token: kubernetes_service.token # encrypted_token and encrypted_token_iv + } ) end - - Gitlab::Database.bulk_insert('cluster_projects', rows_for_cluster_projects) end MigrateKubernetesServiceToNewClustersArchitectures::Service - .where(category: 'deployment', type: 'KubernetesService', template: false) - .each_batch(of: 100) do |batch| - batch.update_all(active: false) - end + .kubernetes_service_without_template.each_batch(of: 100) do |kubernetes_service| + kubernetes_service.update_all(active: false) + end end def down diff --git a/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb b/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb index 5f135ad274e..85ed1328211 100644 --- a/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb +++ b/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb @@ -38,18 +38,6 @@ describe MigrateKubernetesServiceToNewClustersArchitectures, :migration do it_behaves_like 'KubernetesService migration' end - - context 'when KubernetesService is not active' do - let(:active) { false } - - # Platforms::Kubernetes validates `token` reagdless of the activeness, - # whereas KubernetesService validates `token` if only it's activated - # However, in this migration file, there are no validations because of the migration specific model class - # therefore, Validation Error will not happen in this case and just migrate data - let(:token) { '' } - - it_behaves_like 'KubernetesService migration' - end end context 'when unique KubernetesService spawned from Service Template' do @@ -186,6 +174,34 @@ describe MigrateKubernetesServiceToNewClustersArchitectures, :migration do end end + # Platforms::Kubernetes validates `token` reagdless of the activeness, + # whereas KubernetesService validates `token` if only it's activated + # However, in this migration file, there are no validations because of the re-defined model class + # therefore, we should safely add this raw to Platform::Kubernetes + context 'when KubernetesService has empty token' do + let(:project) { create(:project) } + + before do + ActiveRecord::Base.connection.execute <<~SQL + INSERT INTO services (project_id, active, category, type, properties) + VALUES (#{project.id}, false, 'deployment', 'KubernetesService', '{"namespace":"prod","api_url":"http://111.111.111.111","ca_pem":"a","token":""}'); + SQL + end + + it 'does not migrate the KubernetesService and disables the kubernetes_service' do + expect { migrate! }.to change { Clusters::Cluster.count }.by(1) + + project.clusters.last.tap do |cluster| + expect(cluster.environment_scope).to eq('*') + expect(cluster.platform_kubernetes.namespace).to eq('prod') + expect(cluster.platform_kubernetes.api_url).to eq('http://111.111.111.111') + expect(cluster.platform_kubernetes.ca_pem).to eq('a') + expect(cluster.platform_kubernetes.token).to be_empty + expect(project.kubernetes_service).not_to be_active + end + end + end + context 'when KubernetesService does not exist' do let!(:project) { create(:project) } -- cgit v1.2.1 From 54d20d1bad9cfa23bd2935a3f355ac07727bfdab Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Sat, 6 Jan 2018 01:20:13 +0900 Subject: Add changelog --- ...-data-from-kubernetesservice-to-clusters-platforms-kubernetes.yml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelogs/unreleased/40418-migrate-existing-data-from-kubernetesservice-to-clusters-platforms-kubernetes.yml diff --git a/changelogs/unreleased/40418-migrate-existing-data-from-kubernetesservice-to-clusters-platforms-kubernetes.yml b/changelogs/unreleased/40418-migrate-existing-data-from-kubernetesservice-to-clusters-platforms-kubernetes.yml new file mode 100644 index 00000000000..eb7678b6e28 --- /dev/null +++ b/changelogs/unreleased/40418-migrate-existing-data-from-kubernetesservice-to-clusters-platforms-kubernetes.yml @@ -0,0 +1,5 @@ +--- +title: Migrate existing data from KubernetesService to Clusters::Platforms::Kubernetes +merge_request: +author: +type: changed -- cgit v1.2.1 From 290c224847d5623541d4300f7a1bfe2a83bb3b26 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Sat, 6 Jan 2018 01:25:45 +0900 Subject: Fix change log --- ...ing-data-from-kubernetesservice-to-clusters-platforms-kubernetes.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelogs/unreleased/40418-migrate-existing-data-from-kubernetesservice-to-clusters-platforms-kubernetes.yml b/changelogs/unreleased/40418-migrate-existing-data-from-kubernetesservice-to-clusters-platforms-kubernetes.yml index eb7678b6e28..5e158d831a6 100644 --- a/changelogs/unreleased/40418-migrate-existing-data-from-kubernetesservice-to-clusters-platforms-kubernetes.yml +++ b/changelogs/unreleased/40418-migrate-existing-data-from-kubernetesservice-to-clusters-platforms-kubernetes.yml @@ -1,5 +1,5 @@ --- title: Migrate existing data from KubernetesService to Clusters::Platforms::Kubernetes -merge_request: +merge_request: 15589 author: type: changed -- cgit v1.2.1 From 58d074e0013fb6123a084275edd14bb52955c21e Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Sat, 6 Jan 2018 02:16:04 +0900 Subject: Fix StaticSnalysys --- ...24104327_migrate_kubernetes_service_to_new_clusters_architectures.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb b/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb index 27d56fe5b42..0445044cc3c 100644 --- a/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb +++ b/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb @@ -117,7 +117,7 @@ class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migrati MigrateKubernetesServiceToNewClustersArchitectures::Service .kubernetes_service_without_template.each_batch(of: 100) do |kubernetes_service| - kubernetes_service.update_all(active: false) + kubernetes_service.update_all(active: false) end end -- cgit v1.2.1 From df658c7b1337afc71d9f3d66d3e8ad55db26b523 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Mon, 8 Jan 2018 16:17:35 +0900 Subject: Disable STI of ActiveRecord. Refactoring specs. --- ...rnetes_service_to_new_clusters_architectures.rb | 22 +- ...s_service_to_new_clusters_architectures_spec.rb | 228 +++++++++++++++------ 2 files changed, 184 insertions(+), 66 deletions(-) diff --git a/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb b/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb index 0445044cc3c..3fe0a4941d5 100644 --- a/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb +++ b/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb @@ -12,6 +12,7 @@ class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migrati has_many :cluster_projects, class_name: 'MigrateKubernetesServiceToNewClustersArchitectures::ClustersProject' has_many :clusters, through: :cluster_projects, class_name: 'MigrateKubernetesServiceToNewClustersArchitectures::Cluster' has_many :services, class_name: 'MigrateKubernetesServiceToNewClustersArchitectures::Service' + has_one :kubernetes_service, -> { where(category: 'deployment', type: 'KubernetesService') }, class_name: 'MigrateKubernetesServiceToNewClustersArchitectures::Service', inverse_of: :project, foreign_key: :project_id end class Cluster < ActiveRecord::Base @@ -55,8 +56,9 @@ class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migrati include EachBatch self.table_name = 'services' + self.inheritance_column = :_type_disabled # Disable STI, otherwise KubernetesModel will be looked up - belongs_to :project, class_name: 'MigrateKubernetesServiceToNewClustersArchitectures::Project' + belongs_to :project, class_name: 'MigrateKubernetesServiceToNewClustersArchitectures::Project', foreign_key: :project_id scope :unmanaged_kubernetes_service, -> do joins('LEFT JOIN projects ON projects.id = services.project_id') @@ -72,6 +74,22 @@ class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migrati scope :kubernetes_service_without_template, -> do where(category: 'deployment', type: 'KubernetesService', template: false) end + + def api_url + JSON.parse(self.properties)['api_url'] + end + + def ca_pem + JSON.parse(self.properties)['ca_pem'] + end + + def namespace + JSON.parse(self.properties)['namespace'] + end + + def token + JSON.parse(self.properties)['token'] + end end def find_dedicated_environement_scope(project) @@ -101,7 +119,7 @@ class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migrati name: DEFAULT_KUBERNETES_SERVICE_CLUSTER_NAME, provider_type: MigrateKubernetesServiceToNewClustersArchitectures::Cluster.provider_types[:user], platform_type: MigrateKubernetesServiceToNewClustersArchitectures::Cluster.platform_types[:kubernetes], - projects: [kubernetes_service.project.becomes(MigrateKubernetesServiceToNewClustersArchitectures::Project)], + projects: [kubernetes_service.project], environment_scope: find_dedicated_environement_scope(kubernetes_service.project), platform_kubernetes_attributes: { api_url: kubernetes_service.api_url, diff --git a/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb b/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb index 85ed1328211..54ae4c97d4e 100644 --- a/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb +++ b/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb @@ -5,27 +5,32 @@ describe MigrateKubernetesServiceToNewClustersArchitectures, :migration do context 'when unique KubernetesService exists' do shared_examples 'KubernetesService migration' do let(:sample_num) { 2 } - let(:projects) { create_list(:project, sample_num) } + + let(:projects) do + (1..sample_num).each_with_object([]) do |n, array| + array << MigrateKubernetesServiceToNewClustersArchitectures::Project.create! + end + end let!(:kubernetes_services) do projects.map do |project| - create(:kubernetes_service, - project: project, - active: active, - api_url: "https://kubernetes#{project.id}.com", - token: defined?(token) ? token : "token#{project.id}", - ca_pem: "ca_pem#{project.id}") + MigrateKubernetesServiceToNewClustersArchitectures::Service.create!( + project: project, + active: active, + category: 'deployment', + type: 'KubernetesService', + properties: "{\"namespace\":\"prod\",\"api_url\":\"https://kubernetes#{project.id}.com\",\"ca_pem\":\"ca_pem#{project.id}\",\"token\":\"token#{project.id}\"}") end end it 'migrates the KubernetesService to Platform::Kubernetes' do - expect { migrate! }.to change { Clusters::Cluster.count }.by(sample_num) + expect { migrate! }.to change { MigrateKubernetesServiceToNewClustersArchitectures::Cluster.count }.by(sample_num) projects.each do |project| project.clusters.last.tap do |cluster| expect(cluster.enabled).to eq(active) expect(cluster.platform_kubernetes.api_url).to eq(project.kubernetes_service.api_url) - expect(cluster.platform_kubernetes.ca_pem).to eq(project.kubernetes_service.ca_pem) + expect(cluster.platform_kubernetes.ca_cert).to eq(project.kubernetes_service.ca_pem) expect(cluster.platform_kubernetes.token).to eq(project.kubernetes_service.token) expect(project.kubernetes_service).not_to be_active end @@ -42,34 +47,38 @@ describe MigrateKubernetesServiceToNewClustersArchitectures, :migration do context 'when unique KubernetesService spawned from Service Template' do let(:sample_num) { 2 } - let(:projects) { create_list(:project, sample_num) } + + let(:projects) do + (1..sample_num).each_with_object([]) do |n, array| + array << MigrateKubernetesServiceToNewClustersArchitectures::Project.create! + end + end let!(:kubernetes_service_template) do - create(:kubernetes_service, - project: nil, - template: true, - api_url: "https://sample.kubernetes.com", - token: "token-sample", - ca_pem: "ca_pem-sample") + MigrateKubernetesServiceToNewClustersArchitectures::Service.create!( + template: true, + category: 'deployment', + type: 'KubernetesService', + properties: "{\"namespace\":\"prod\",\"api_url\":\"https://sample.kubernetes.com\",\"ca_pem\":\"ca_pem-sample\",\"token\":\"token-sample\"}") end let!(:kubernetes_services) do projects.map do |project| - create(:kubernetes_service, - project: project, - api_url: kubernetes_service_template.api_url, - token: kubernetes_service_template.token, - ca_pem: kubernetes_service_template.ca_pem) + MigrateKubernetesServiceToNewClustersArchitectures::Service.create!( + project: project, + category: 'deployment', + type: 'KubernetesService', + properties: "{\"namespace\":\"prod\",\"api_url\":\"#{kubernetes_service_template.api_url}\",\"ca_pem\":\"#{kubernetes_service_template.ca_pem}\",\"token\":\"#{kubernetes_service_template.token}\"}") end end it 'migrates the KubernetesService to Platform::Kubernetes without template' do - expect { migrate! }.to change { Clusters::Cluster.count }.by(sample_num) + expect { migrate! }.to change { MigrateKubernetesServiceToNewClustersArchitectures::Cluster.count }.by(sample_num) projects.each do |project| project.clusters.last.tap do |cluster| expect(cluster.platform_kubernetes.api_url).to eq(project.kubernetes_service.api_url) - expect(cluster.platform_kubernetes.ca_pem).to eq(project.kubernetes_service.ca_pem) + expect(cluster.platform_kubernetes.ca_cert).to eq(project.kubernetes_service.ca_pem) expect(cluster.platform_kubernetes.token).to eq(project.kubernetes_service.token) expect(project.kubernetes_service).not_to be_active end @@ -78,21 +87,32 @@ describe MigrateKubernetesServiceToNewClustersArchitectures, :migration do end context 'when managed KubernetesService exists' do - let(:project) { create(:project) } - let(:cluster) { create(:cluster, :provided_by_gcp, projects: [project]) } - let!(:platform_kubernetes) { cluster.platform_kubernetes } + let(:project) { MigrateKubernetesServiceToNewClustersArchitectures::Project.create! } + + let(:cluster) do + MigrateKubernetesServiceToNewClustersArchitectures::Cluster.create!( + projects: [project], + name: 'sample-cluster', + platform_type: :kubernetes, + provider_type: :user, + platform_kubernetes_attributes: { + api_url: 'https://sample.kubernetes.com', + ca_cert: 'ca_pem-sample', + token: 'token-sample' + } ) + end let!(:kubernetes_service) do - create(:kubernetes_service, - project: project, - active: cluster.enabled, - api_url: platform_kubernetes.api_url, - token: platform_kubernetes.token, - ca_pem: platform_kubernetes.ca_cert) + MigrateKubernetesServiceToNewClustersArchitectures::Service.create!( + project: project, + active: cluster.enabled, + category: 'deployment', + type: 'KubernetesService', + properties: "{\"api_url\":\"#{cluster.platform_kubernetes.api_url}\"}") end it 'does not migrate the KubernetesService and disables the kubernetes_service' do # Because the corresponding Platform::Kubernetes already exists - expect { migrate! }.not_to change { Clusters::Cluster.count } + expect { migrate! }.not_to change { MigrateKubernetesServiceToNewClustersArchitectures::Cluster.count } kubernetes_service.reload expect(kubernetes_service).not_to be_active @@ -100,18 +120,39 @@ describe MigrateKubernetesServiceToNewClustersArchitectures, :migration do end context 'when production cluster has already been existed' do # i.e. There are no environment_scope conflicts - let(:project) { create(:project) } - let!(:cluster) { create(:cluster, :provided_by_gcp, environment_scope: 'production/*', projects: [project]) } - let!(:kubernetes_service) { create(:kubernetes_service, api_url: 'https://debug.kube.com', active: true, project: project) } + let(:project) { MigrateKubernetesServiceToNewClustersArchitectures::Project.create! } + + let(:cluster) do + MigrateKubernetesServiceToNewClustersArchitectures::Cluster.create!( + projects: [project], + name: 'sample-cluster', + platform_type: :kubernetes, + provider_type: :user, + environment_scope: 'production/*', + platform_kubernetes_attributes: { + api_url: 'https://sample.kubernetes.com', + ca_cert: 'ca_pem-sample', + token: 'token-sample' + } ) + end + + let!(:kubernetes_service) do + MigrateKubernetesServiceToNewClustersArchitectures::Service.create!( + project: project, + active: true, + category: 'deployment', + type: 'KubernetesService', + properties: "{\"api_url\":\"https://debug.kube.com\"}") + end it 'migrates the KubernetesService to Platform::Kubernetes' do - expect { migrate! }.to change { Clusters::Cluster.count }.by(1) + expect { migrate! }.to change { MigrateKubernetesServiceToNewClustersArchitectures::Cluster.count }.by(1) kubernetes_service.reload project.clusters.last.tap do |cluster| expect(cluster.environment_scope).to eq('*') expect(cluster.platform_kubernetes.api_url).to eq(kubernetes_service.api_url) - expect(cluster.platform_kubernetes.ca_pem).to eq(kubernetes_service.ca_pem) + expect(cluster.platform_kubernetes.ca_cert).to eq(kubernetes_service.ca_pem) expect(cluster.platform_kubernetes.token).to eq(kubernetes_service.token) expect(kubernetes_service).not_to be_active end @@ -119,18 +160,39 @@ describe MigrateKubernetesServiceToNewClustersArchitectures, :migration do end context 'when default cluster has already been existed' do - let(:project) { create(:project) } - let!(:cluster) { create(:cluster, :provided_by_gcp, environment_scope: '*', projects: [project]) } - let!(:kubernetes_service) { create(:kubernetes_service, api_url: 'https://debug.kube.com', active: true, project: project) } + let(:project) { MigrateKubernetesServiceToNewClustersArchitectures::Project.create! } + + let!(:cluster) do + MigrateKubernetesServiceToNewClustersArchitectures::Cluster.create!( + projects: [project], + name: 'sample-cluster', + platform_type: :kubernetes, + provider_type: :user, + environment_scope: '*', + platform_kubernetes_attributes: { + api_url: 'https://sample.kubernetes.com', + ca_cert: 'ca_pem-sample', + token: 'token-sample' + } ) + end + + let!(:kubernetes_service) do + MigrateKubernetesServiceToNewClustersArchitectures::Service.create!( + project: project, + active: true, + category: 'deployment', + type: 'KubernetesService', + properties: "{\"api_url\":\"https://debug.kube.com\"}") + end it 'migrates the KubernetesService to Platform::Kubernetes with dedicated environment_scope' do # Because environment_scope is duplicated - expect { migrate! }.to change { Clusters::Cluster.count }.by(1) + expect { migrate! }.to change { MigrateKubernetesServiceToNewClustersArchitectures::Cluster.count }.by(1) kubernetes_service.reload project.clusters.last.tap do |cluster| expect(cluster.environment_scope).to eq('migrated/*') expect(cluster.platform_kubernetes.api_url).to eq(kubernetes_service.api_url) - expect(cluster.platform_kubernetes.ca_pem).to eq(kubernetes_service.ca_pem) + expect(cluster.platform_kubernetes.ca_cert).to eq(kubernetes_service.ca_pem) expect(cluster.platform_kubernetes.token).to eq(kubernetes_service.token) expect(kubernetes_service).not_to be_active end @@ -138,19 +200,53 @@ describe MigrateKubernetesServiceToNewClustersArchitectures, :migration do end context 'when default cluster and migrated cluster has already been existed' do - let(:project) { create(:project) } - let!(:cluster) { create(:cluster, :provided_by_gcp, environment_scope: '*', projects: [project]) } - let!(:migrated_cluster) { create(:cluster, :provided_by_gcp, environment_scope: 'migrated/*', projects: [project]) } - let!(:kubernetes_service) { create(:kubernetes_service, api_url: 'https://debug.kube.com', active: true, project: project) } + let(:project) { MigrateKubernetesServiceToNewClustersArchitectures::Project.create! } + + let!(:cluster) do + MigrateKubernetesServiceToNewClustersArchitectures::Cluster.create!( + projects: [project], + name: 'sample-cluster', + platform_type: :kubernetes, + provider_type: :user, + environment_scope: '*', + platform_kubernetes_attributes: { + api_url: 'https://sample.kubernetes.com', + ca_cert: 'ca_pem-sample', + token: 'token-sample' + } ) + end + + let!(:migrated_cluster) do + MigrateKubernetesServiceToNewClustersArchitectures::Cluster.create!( + projects: [project], + name: 'sample-cluster', + platform_type: :kubernetes, + provider_type: :user, + environment_scope: 'migrated/*', + platform_kubernetes_attributes: { + api_url: 'https://sample.kubernetes.com', + ca_cert: 'ca_pem-sample', + token: 'token-sample' + } ) + end + + let!(:kubernetes_service) do + MigrateKubernetesServiceToNewClustersArchitectures::Service.create!( + project: project, + active: true, + category: 'deployment', + type: 'KubernetesService', + properties: "{\"api_url\":\"https://debug.kube.com\"}") + end it 'migrates the KubernetesService to Platform::Kubernetes with dedicated environment_scope' do # Because environment_scope is duplicated - expect { migrate! }.to change { Clusters::Cluster.count }.by(1) + expect { migrate! }.to change { MigrateKubernetesServiceToNewClustersArchitectures::Cluster.count }.by(1) kubernetes_service.reload project.clusters.last.tap do |cluster| expect(cluster.environment_scope).to eq('migrated0/*') expect(cluster.platform_kubernetes.api_url).to eq(kubernetes_service.api_url) - expect(cluster.platform_kubernetes.ca_pem).to eq(kubernetes_service.ca_pem) + expect(cluster.platform_kubernetes.ca_cert).to eq(kubernetes_service.ca_pem) expect(cluster.platform_kubernetes.token).to eq(kubernetes_service.token) expect(kubernetes_service).not_to be_active end @@ -158,17 +254,19 @@ describe MigrateKubernetesServiceToNewClustersArchitectures, :migration do end context 'when KubernetesService has nullified parameters' do - let(:project) { create(:project) } + let(:project) { MigrateKubernetesServiceToNewClustersArchitectures::Project.create! } before do - ActiveRecord::Base.connection.execute <<~SQL - INSERT INTO services (project_id, active, category, type, properties) - VALUES (#{project.id}, false, 'deployment', 'KubernetesService', '{}'); - SQL + MigrateKubernetesServiceToNewClustersArchitectures::Service.create!( + project: project, + active: false, + category: 'deployment', + type: 'KubernetesService', + properties: "{}") end it 'does not migrate the KubernetesService and disables the kubernetes_service' do - expect { migrate! }.not_to change { Clusters::Cluster.count } + expect { migrate! }.not_to change { MigrateKubernetesServiceToNewClustersArchitectures::Cluster.count } expect(project.kubernetes_service).not_to be_active end @@ -179,23 +277,25 @@ describe MigrateKubernetesServiceToNewClustersArchitectures, :migration do # However, in this migration file, there are no validations because of the re-defined model class # therefore, we should safely add this raw to Platform::Kubernetes context 'when KubernetesService has empty token' do - let(:project) { create(:project) } + let(:project) { MigrateKubernetesServiceToNewClustersArchitectures::Project.create! } before do - ActiveRecord::Base.connection.execute <<~SQL - INSERT INTO services (project_id, active, category, type, properties) - VALUES (#{project.id}, false, 'deployment', 'KubernetesService', '{"namespace":"prod","api_url":"http://111.111.111.111","ca_pem":"a","token":""}'); - SQL + MigrateKubernetesServiceToNewClustersArchitectures::Service.create!( + project: project, + active: false, + category: 'deployment', + type: 'KubernetesService', + properties: "{\"namespace\":\"prod\",\"api_url\":\"http://111.111.111.111\",\"ca_pem\":\"a\",\"token\":\"\"}") end it 'does not migrate the KubernetesService and disables the kubernetes_service' do - expect { migrate! }.to change { Clusters::Cluster.count }.by(1) + expect { migrate! }.to change { MigrateKubernetesServiceToNewClustersArchitectures::Cluster.count }.by(1) project.clusters.last.tap do |cluster| expect(cluster.environment_scope).to eq('*') expect(cluster.platform_kubernetes.namespace).to eq('prod') expect(cluster.platform_kubernetes.api_url).to eq('http://111.111.111.111') - expect(cluster.platform_kubernetes.ca_pem).to eq('a') + expect(cluster.platform_kubernetes.ca_cert).to eq('a') expect(cluster.platform_kubernetes.token).to be_empty expect(project.kubernetes_service).not_to be_active end @@ -203,10 +303,10 @@ describe MigrateKubernetesServiceToNewClustersArchitectures, :migration do end context 'when KubernetesService does not exist' do - let!(:project) { create(:project) } + let!(:project) { MigrateKubernetesServiceToNewClustersArchitectures::Project.create! } it 'does not migrate the KubernetesService' do - expect { migrate! }.not_to change { Clusters::Cluster.count } + expect { migrate! }.not_to change { MigrateKubernetesServiceToNewClustersArchitectures::Cluster.count } end end end -- cgit v1.2.1 From c425ff750179542a94da69af3a507c70cd77ca48 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Mon, 8 Jan 2018 17:01:29 +0900 Subject: Fix static analysys --- ...netes_service_to_new_clusters_architectures_spec.rb | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb b/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb index 54ae4c97d4e..df0015b6dd3 100644 --- a/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb +++ b/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb @@ -91,15 +91,15 @@ describe MigrateKubernetesServiceToNewClustersArchitectures, :migration do let(:cluster) do MigrateKubernetesServiceToNewClustersArchitectures::Cluster.create!( - projects: [project], - name: 'sample-cluster', - platform_type: :kubernetes, - provider_type: :user, - platform_kubernetes_attributes: { - api_url: 'https://sample.kubernetes.com', - ca_cert: 'ca_pem-sample', - token: 'token-sample' - } ) + projects: [project], + name: 'sample-cluster', + platform_type: :kubernetes, + provider_type: :user, + platform_kubernetes_attributes: { + api_url: 'https://sample.kubernetes.com', + ca_cert: 'ca_pem-sample', + token: 'token-sample' + } ) end let!(:kubernetes_service) do -- cgit v1.2.1 From 66ebb206b09f83288ee6acbd6d11b2963ae43c9b Mon Sep 17 00:00:00 2001 From: Christiaan Van den Poel Date: Mon, 8 Jan 2018 09:06:25 +0000 Subject: disables the shortcut to the issue boards when issues are disabled --- app/controllers/projects/application_controller.rb | 4 +++ app/controllers/projects/boards_controller.rb | 1 + app/controllers/projects/issues_controller.rb | 4 --- app/views/layouts/nav/sidebar/_project.html.haml | 12 +++++--- ...ssue-board-shortcut-working-while-no-issues.yml | 5 +++ .../controllers/projects/boards_controller_spec.rb | 10 ++++++ spec/features/boards/keyboard_shortcut_spec.rb | 36 ++++++++++++++++------ spec/features/issues/keyboard_shortcut_spec.rb | 36 ++++++++++++++++++++++ 8 files changed, 90 insertions(+), 18 deletions(-) create mode 100644 changelogs/unreleased/41244-issue-board-shortcut-working-while-no-issues.yml create mode 100644 spec/features/issues/keyboard_shortcut_spec.rb diff --git a/app/controllers/projects/application_controller.rb b/app/controllers/projects/application_controller.rb index 9e79852e378..6025a40348b 100644 --- a/app/controllers/projects/application_controller.rb +++ b/app/controllers/projects/application_controller.rb @@ -86,4 +86,8 @@ class Projects::ApplicationController < ApplicationController def require_pages_enabled! not_found unless @project.pages_available? end + + def check_issues_available! + return render_404 unless @project.feature_available?(:issues, current_user) + end end diff --git a/app/controllers/projects/boards_controller.rb b/app/controllers/projects/boards_controller.rb index e36105ddc11..949e54ff819 100644 --- a/app/controllers/projects/boards_controller.rb +++ b/app/controllers/projects/boards_controller.rb @@ -2,6 +2,7 @@ class Projects::BoardsController < Projects::ApplicationController include BoardsResponses include IssuableCollections + before_action :check_issues_available! before_action :authorize_read_board!, only: [:index, :show] before_action :assign_endpoint_vars diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index d7a3441a245..384f18b316c 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -194,10 +194,6 @@ class Projects::IssuesController < Projects::ApplicationController render_404 unless can?(current_user, :push_code, @project) && @issue.can_be_worked_on?(current_user) end - def check_issues_available! - return render_404 unless @project.feature_available?(:issues, current_user) - end - def render_issue_json if @issue.valid? render json: serializer.represent(@issue) diff --git a/app/views/layouts/nav/sidebar/_project.html.haml b/app/views/layouts/nav/sidebar/_project.html.haml index be39f577ba7..1fa3a3041fd 100644 --- a/app/views/layouts/nav/sidebar/_project.html.haml +++ b/app/views/layouts/nav/sidebar/_project.html.haml @@ -299,9 +299,10 @@ Charts -# Shortcut to Issues > New Issue - %li.hidden - = link_to new_project_issue_path(@project), class: 'shortcuts-new-issue' do - Create a new issue + - if project_nav_tab?(:issues) + %li.hidden + = link_to new_project_issue_path(@project), class: 'shortcuts-new-issue' do + Create a new issue -# Shortcut to Pipelines > Jobs - if project_nav_tab? :builds @@ -316,5 +317,6 @@ Commits -# Shortcut to issue boards - %li.hidden - = link_to 'Issue Boards', project_boards_path(@project), title: 'Issue Boards', class: 'shortcuts-issue-boards' + - if project_nav_tab?(:issues) + %li.hidden + = link_to 'Issue Boards', project_boards_path(@project), title: 'Issue Boards', class: 'shortcuts-issue-boards' diff --git a/changelogs/unreleased/41244-issue-board-shortcut-working-while-no-issues.yml b/changelogs/unreleased/41244-issue-board-shortcut-working-while-no-issues.yml new file mode 100644 index 00000000000..b2c3a86551b --- /dev/null +++ b/changelogs/unreleased/41244-issue-board-shortcut-working-while-no-issues.yml @@ -0,0 +1,5 @@ +--- +title: disables shortcut to issue boards when issues are not enabled +merge_request: 16020 +author: Christiaan Van den Poel +type: fixed diff --git a/spec/controllers/projects/boards_controller_spec.rb b/spec/controllers/projects/boards_controller_spec.rb index 305af289531..4d765229bde 100644 --- a/spec/controllers/projects/boards_controller_spec.rb +++ b/spec/controllers/projects/boards_controller_spec.rb @@ -55,6 +55,16 @@ describe Projects::BoardsController do end end + context 'issues are disabled' do + let(:project) { create(:project, :issues_disabled) } + + it 'returns a not found 404 response' do + list_boards + + expect(response).to have_gitlab_http_status(404) + end + end + def list_boards(format: :html) get :index, namespace_id: project.namespace, project_id: project, diff --git a/spec/features/boards/keyboard_shortcut_spec.rb b/spec/features/boards/keyboard_shortcut_spec.rb index 435de3861cf..d820a59aa16 100644 --- a/spec/features/boards/keyboard_shortcut_spec.rb +++ b/spec/features/boards/keyboard_shortcut_spec.rb @@ -1,20 +1,38 @@ require 'rails_helper' describe 'Issue Boards shortcut', :js do - let(:project) { create(:project) } + context 'issues are enabled' do + let(:project) { create(:project) } - before do - create(:board, project: project) + before do + create(:board, project: project) - sign_in(create(:admin)) + sign_in(create(:admin)) - visit project_path(project) + visit project_path(project) + end + + it 'takes user to issue board index' do + find('body').native.send_keys('gb') + expect(page).to have_selector('.boards-list') + + wait_for_requests + end end - it 'takes user to issue board index' do - find('body').native.send_keys('gb') - expect(page).to have_selector('.boards-list') + context 'issues are not enabled' do + let(:project) { create(:project, :issues_disabled) } + + before do + sign_in(create(:admin)) + + visit project_path(project) + end + + it 'does not take user to the issue board index' do + find('body').native.send_keys('gb') - wait_for_requests + expect(page).to have_selector("body[data-page='projects:show']") + end end end diff --git a/spec/features/issues/keyboard_shortcut_spec.rb b/spec/features/issues/keyboard_shortcut_spec.rb new file mode 100644 index 00000000000..961de9d3d25 --- /dev/null +++ b/spec/features/issues/keyboard_shortcut_spec.rb @@ -0,0 +1,36 @@ +require 'rails_helper' + +describe 'Issues shortcut', :js do + context 'New Issue shortcut' do + context 'issues are enabled' do + let(:project) { create(:project) } + + before do + sign_in(create(:admin)) + + visit project_path(project) + end + + it 'takes user to the new issue page' do + find('body').native.send_keys('i') + expect(page).to have_selector('#new_issue') + end + end + + context 'issues are not enabled' do + let(:project) { create(:project, :issues_disabled) } + + before do + sign_in(create(:admin)) + + visit project_path(project) + end + + it 'does not take user to the new issue page' do + find('body').native.send_keys('i') + + expect(page).to have_selector("body[data-page='projects:show']") + end + end + end +end -- cgit v1.2.1 From f07727d92edab71444e28b12f4d8b6d9f8db690c Mon Sep 17 00:00:00 2001 From: Maurizio De Santis Date: Mon, 8 Jan 2018 09:27:50 +0000 Subject: Fix slash commands dropdown description --- app/assets/javascripts/gfm_auto_complete.js | 6 +++--- app/assets/stylesheets/framework/markdown_area.scss | 12 ++++++++++++ ...mmands-dropdown-description-mis-alignement-on-firefox.yml | 5 +++++ 3 files changed, 20 insertions(+), 3 deletions(-) create mode 100644 changelogs/unreleased/40190-fix-slash-commands-dropdown-description-mis-alignement-on-firefox.yml diff --git a/app/assets/javascripts/gfm_auto_complete.js b/app/assets/javascripts/gfm_auto_complete.js index d918d80df8d..df20e1e9c88 100644 --- a/app/assets/javascripts/gfm_auto_complete.js +++ b/app/assets/javascripts/gfm_auto_complete.js @@ -57,12 +57,12 @@ class GfmAutoComplete { displayTpl(value) { if (GfmAutoComplete.isLoading(value)) return GfmAutoComplete.Loading.template; // eslint-disable-next-line no-template-curly-in-string - let tpl = '
  • /${name}'; + let tpl = '
  • /${name}'; if (value.aliases.length > 0) { - tpl += ' (or /<%- aliases.join(", /") %>)'; + tpl += ' (or /<%- aliases.join(", /") %>)'; } if (value.params.length > 0) { - tpl += ' <%- params.join(" ") %>'; + tpl += ' <%- params.join(" ") %>'; } if (value.description !== '') { tpl += '<%- description %>'; diff --git a/app/assets/stylesheets/framework/markdown_area.scss b/app/assets/stylesheets/framework/markdown_area.scss index 6b07ffdbd61..938f5f49c09 100644 --- a/app/assets/stylesheets/framework/markdown_area.scss +++ b/app/assets/stylesheets/framework/markdown_area.scss @@ -192,6 +192,17 @@ overflow-y: auto; overflow-x: hidden; + .name, + small.aliases, + small.params { + float: left; + } + + small.aliases, + small.params { + padding: 2px 5px; + } + small.description { float: right; padding: 3px 5px; @@ -209,6 +220,7 @@ } ul > li { + @include clearfix; white-space: nowrap; } diff --git a/changelogs/unreleased/40190-fix-slash-commands-dropdown-description-mis-alignement-on-firefox.yml b/changelogs/unreleased/40190-fix-slash-commands-dropdown-description-mis-alignement-on-firefox.yml new file mode 100644 index 00000000000..71a606ff607 --- /dev/null +++ b/changelogs/unreleased/40190-fix-slash-commands-dropdown-description-mis-alignement-on-firefox.yml @@ -0,0 +1,5 @@ +--- +title: "Fix slash commands dropdown description mis-alignment on Firefox" +merge_request: 16125 +author: Maurizio De Santis +type: fixed -- cgit v1.2.1 From 1dc051fb62623723448f50004643e4b45468820b Mon Sep 17 00:00:00 2001 From: Mike Greiling Date: Mon, 8 Jan 2018 09:35:23 +0000 Subject: Resolve "Add graph value to hover" --- .../javascripts/monitoring/components/graph.vue | 49 ++--- .../monitoring/components/graph/deployment.vue | 129 +------------ .../monitoring/components/graph/flag.vue | 205 ++++++++++++++++----- .../monitoring/mixins/monitoring_mixins.js | 5 +- .../monitoring/utils/date_time_formatters.js | 2 +- app/assets/stylesheets/pages/environments.scss | 67 +++++++ .../unreleased/38030-add-graph-value-to-hover.yml | 5 + .../monitoring/graph/deployment_spec.js | 144 +-------------- spec/javascripts/monitoring/graph/flag_spec.js | 114 ++++++++++-- 9 files changed, 364 insertions(+), 356 deletions(-) create mode 100644 changelogs/unreleased/38030-add-graph-value-to-hover.yml diff --git a/app/assets/javascripts/monitoring/components/graph.vue b/app/assets/javascripts/monitoring/components/graph.vue index eede04a06cd..a50b80c23d0 100644 --- a/app/assets/javascripts/monitoring/components/graph.vue +++ b/app/assets/javascripts/monitoring/components/graph.vue @@ -69,8 +69,8 @@ currentFlagPosition: 0, showFlag: false, showFlagContent: false, - showDeployInfo: true, timeSeries: [], + realPixelRatio: 1, }; }, @@ -87,10 +87,7 @@ }, innerViewBox() { - if ((this.baseGraphWidth - 150) > 0) { - return `0 0 ${this.baseGraphWidth - 150} ${this.baseGraphHeight}`; - } - return '0 0 0 0'; + return `0 0 ${this.baseGraphWidth - 150} ${this.baseGraphHeight}`; }, axisTransform() { @@ -102,6 +99,10 @@ paddingBottom: `${(Math.ceil(this.baseGraphHeight * 100) / this.baseGraphWidth) || 0}%`, }; }, + + deploymentFlagData() { + return this.reducedDeploymentData.find(deployment => deployment.showDeploymentFlag); + }, }, methods: { @@ -122,6 +123,10 @@ this.graphHeight = this.graphHeight - this.margin.top - this.margin.bottom; this.baseGraphHeight = this.graphHeight; this.baseGraphWidth = this.graphWidth; + + // pixel offsets inside the svg and outside are not 1:1 + this.realPixelRatio = (this.$refs.baseSvg.clientWidth / this.baseGraphWidth); + this.renderAxesPaths(); this.formatDeployments(); }, @@ -261,6 +266,11 @@ :line-color="path.lineColor" :area-color="path.areaColor" /> + - - + diff --git a/app/assets/javascripts/monitoring/components/graph/deployment.vue b/app/assets/javascripts/monitoring/components/graph/deployment.vue index 026e2fd0c49..8d6393d4ce5 100644 --- a/app/assets/javascripts/monitoring/components/graph/deployment.vue +++ b/app/assets/javascripts/monitoring/components/graph/deployment.vue @@ -1,13 +1,6 @@ diff --git a/app/assets/javascripts/monitoring/mixins/monitoring_mixins.js b/app/assets/javascripts/monitoring/mixins/monitoring_mixins.js index cbca14ede02..6cc67ba57ee 100644 --- a/app/assets/javascripts/monitoring/mixins/monitoring_mixins.js +++ b/app/assets/javascripts/monitoring/mixins/monitoring_mixins.js @@ -29,15 +29,18 @@ const mixins = { time.setSeconds(this.timeSeries[0].values[0].time.getSeconds()); if (xPos >= 0) { + const seriesIndex = bisectDate(this.timeSeries[0].values, time, 1); + deploymentDataArray.push({ id: deployment.id, time, sha: deployment.sha, commitUrl: `${this.projectPath}/commit/${deployment.sha}`, tag: deployment.tag, - tagUrl: `${this.tagsPath}/${deployment.tag}`, + tagUrl: deployment.tag ? `${this.tagsPath}/${deployment.ref.name}` : null, ref: deployment.ref.name, xPos, + seriesIndex, showDeploymentFlag: false, }); } diff --git a/app/assets/javascripts/monitoring/utils/date_time_formatters.js b/app/assets/javascripts/monitoring/utils/date_time_formatters.js index 068813ddee6..f3c9acdd93e 100644 --- a/app/assets/javascripts/monitoring/utils/date_time_formatters.js +++ b/app/assets/javascripts/monitoring/utils/date_time_formatters.js @@ -14,7 +14,7 @@ const d3 = { timeYear, }; -export const dateFormat = d3.time('%b %-d, %Y'); +export const dateFormat = d3.time('%a, %b %-d'); export const timeFormat = d3.time('%-I:%M%p'); export const dateFormatWithName = d3.time('%a, %b %-d'); export const bisectDate = d3.bisector(d => d.time).left; diff --git a/app/assets/stylesheets/pages/environments.scss b/app/assets/stylesheets/pages/environments.scss index f4882305c57..794bc668562 100644 --- a/app/assets/stylesheets/pages/environments.scss +++ b/app/assets/stylesheets/pages/environments.scss @@ -248,6 +248,73 @@ } } +.prometheus-graph-cursor { + position: absolute; + background: $theme-gray-600; + width: 1px; +} + +.prometheus-graph-flag { + display: block; + min-width: 160px; + + h5 { + padding: 0; + margin: 0; + font-size: 14px; + line-height: 1.2; + } + + table { + border-collapse: collapse; + padding: 0; + margin: 0; + } + + td { + vertical-align: middle; + + + td { + padding-left: 5px; + vertical-align: top; + } + } + + .deploy-meta-content { + border-bottom: 1px solid $white-dark; + + svg { + height: 15px; + vertical-align: bottom; + } + } + + &.popover { + &.left { + left: auto; + right: 0; + margin-right: 10px; + } + + &.right { + left: 0; + right: auto; + margin-left: 10px; + } + + > .arrow { + top: 40px; + } + + > .popover-title, + > .popover-content { + padding: 5px 8px; + font-size: 12px; + white-space: nowrap; + } + } +} + .prometheus-svg-container { position: relative; height: 0; diff --git a/changelogs/unreleased/38030-add-graph-value-to-hover.yml b/changelogs/unreleased/38030-add-graph-value-to-hover.yml new file mode 100644 index 00000000000..233db2b19c9 --- /dev/null +++ b/changelogs/unreleased/38030-add-graph-value-to-hover.yml @@ -0,0 +1,5 @@ +--- +title: Display graph values on hover within monitoring page +merge_request: 16261 +author: +type: changed diff --git a/spec/javascripts/monitoring/graph/deployment_spec.js b/spec/javascripts/monitoring/graph/deployment_spec.js index bf6ada8185e..d07db871d69 100644 --- a/spec/javascripts/monitoring/graph/deployment_spec.js +++ b/spec/javascripts/monitoring/graph/deployment_spec.js @@ -11,168 +11,38 @@ const createComponent = (propsData) => { }; describe('MonitoringDeployment', () => { - const reducedDeploymentData = [deploymentData[0]]; - reducedDeploymentData[0].ref = reducedDeploymentData[0].ref.name; - reducedDeploymentData[0].xPos = 10; - reducedDeploymentData[0].time = new Date(reducedDeploymentData[0].created_at); describe('Methods', () => { - it('refText shows the ref when a tag is available', () => { - reducedDeploymentData[0].tag = '1.0'; - const component = createComponent({ - showDeployInfo: false, - deploymentData: reducedDeploymentData, - graphWidth: 440, - graphHeight: 300, - graphHeightOffset: 120, - }); - - expect( - component.refText(reducedDeploymentData[0]), - ).toEqual(reducedDeploymentData[0].ref); - }); - - it('refText shows the sha when no tag is available', () => { - reducedDeploymentData[0].tag = null; - const component = createComponent({ - showDeployInfo: false, - deploymentData: reducedDeploymentData, - graphHeight: 300, - graphWidth: 440, - graphHeightOffset: 120, - }); - - expect( - component.refText(reducedDeploymentData[0]), - ).toContain('f5bcd1'); - }); - - it('nameDeploymentClass creates a class with the prefix deploy-info-', () => { + it('should contain a hidden gradient', () => { const component = createComponent({ - showDeployInfo: false, - deploymentData: reducedDeploymentData, + showDeployInfo: true, + deploymentData, graphHeight: 300, graphWidth: 440, graphHeightOffset: 120, }); - expect( - component.nameDeploymentClass(reducedDeploymentData[0]), - ).toContain('deploy-info'); + expect(component.$el.querySelector('#shadow-gradient')).not.toBeNull(); }); it('transformDeploymentGroup translates an available deployment', () => { const component = createComponent({ showDeployInfo: false, - deploymentData: reducedDeploymentData, + deploymentData, graphHeight: 300, graphWidth: 440, graphHeightOffset: 120, }); expect( - component.transformDeploymentGroup(reducedDeploymentData[0]), + component.transformDeploymentGroup({ xPos: 16 }), ).toContain('translate(11, 20)'); }); - it('hides the deployment flag', () => { - reducedDeploymentData[0].showDeploymentFlag = false; - const component = createComponent({ - showDeployInfo: true, - deploymentData: reducedDeploymentData, - graphWidth: 440, - graphHeight: 300, - graphHeightOffset: 120, - }); - - expect(component.$el.querySelector('.js-deploy-info-box')).toBeNull(); - }); - - it('positions the flag to the left when the xPos is too far right', () => { - reducedDeploymentData[0].showDeploymentFlag = false; - reducedDeploymentData[0].xPos = 250; - const component = createComponent({ - showDeployInfo: true, - deploymentData: reducedDeploymentData, - graphWidth: 440, - graphHeight: 300, - graphHeightOffset: 120, - }); - - expect( - component.positionFlag(reducedDeploymentData[0]), - ).toBeLessThan(0); - }); - - it('shows the deployment flag', () => { - reducedDeploymentData[0].showDeploymentFlag = true; - const component = createComponent({ - showDeployInfo: true, - deploymentData: reducedDeploymentData, - graphHeight: 300, - graphWidth: 440, - graphHeightOffset: 120, - }); - - expect( - component.$el.querySelector('.js-deploy-info-box').style.display, - ).not.toEqual('display: none;'); - }); - - it('contains date, refs and the "deployed" text', () => { - reducedDeploymentData[0].showDeploymentFlag = true; - const component = createComponent({ - showDeployInfo: true, - deploymentData: reducedDeploymentData, - graphHeight: 300, - graphWidth: 440, - graphHeightOffset: 120, - }); - - expect( - component.$el.querySelectorAll('.deploy-info-text'), - ).toContainText('Deployed'); - - expect( - component.$el.querySelectorAll('.deploy-info-text'), - ).toContainText('Wed, May 31'); - - expect( - component.$el.querySelectorAll('.deploy-info-text'), - ).toContainText(component.refText(reducedDeploymentData[0])); - }); - - it('contains a link to the commit contents', () => { - reducedDeploymentData[0].showDeploymentFlag = true; - const component = createComponent({ - showDeployInfo: true, - deploymentData: reducedDeploymentData, - graphHeight: 300, - graphWidth: 440, - graphHeightOffset: 120, - }); - - expect( - component.$el.querySelectorAll('.deploy-info-text-link')[0].parentElement.getAttribute('xlink:href'), - ).not.toEqual(''); - }); - - it('should contain a hidden gradient', () => { - const component = createComponent({ - showDeployInfo: true, - deploymentData: reducedDeploymentData, - graphHeight: 300, - graphWidth: 440, - graphHeightOffset: 120, - }); - - expect(component.$el.querySelector('#shadow-gradient')).not.toBeNull(); - }); - describe('Computed props', () => { it('calculatedHeight', () => { const component = createComponent({ showDeployInfo: true, - deploymentData: reducedDeploymentData, + deploymentData, graphHeight: 300, graphWidth: 440, graphHeightOffset: 120, diff --git a/spec/javascripts/monitoring/graph/flag_spec.js b/spec/javascripts/monitoring/graph/flag_spec.js index 8ee1171419d..2d474e9092f 100644 --- a/spec/javascripts/monitoring/graph/flag_spec.js +++ b/spec/javascripts/monitoring/graph/flag_spec.js @@ -1,5 +1,6 @@ import Vue from 'vue'; import GraphFlag from '~/monitoring/components/graph/flag.vue'; +import { deploymentData } from '../mock_data'; const createComponent = (propsData) => { const Component = Vue.extend(GraphFlag); @@ -9,11 +10,6 @@ const createComponent = (propsData) => { }).$mount(); }; -function getCoordinate(component, selector, coordinate) { - const coordinateVal = component.$el.querySelector(selector).getAttribute(coordinate); - return parseInt(coordinateVal, 10); -} - const defaultValuesComponent = { currentXCoordinate: 200, currentYCoordinate: 100, @@ -25,31 +21,111 @@ const defaultValuesComponent = { graphHeight: 300, graphHeightOffset: 120, showFlagContent: true, + realPixelRatio: 1, + timeSeries: [{ + values: [{ + time: new Date('2017-06-04T18:17:33.501Z'), + value: '1.49609375', + }], + }], + unitOfDisplay: 'ms', + currentDataIndex: 0, + legendTitle: 'Average', +}; + +const deploymentFlagData = { + ...deploymentData[0], + ref: deploymentData[0].ref.name, + xPos: 10, + time: new Date(deploymentData[0].created_at), }; describe('GraphFlag', () => { - it('has a line and a circle located at the currentXCoordinate and currentYCoordinate', () => { - const component = createComponent(defaultValuesComponent); + let component; - expect(getCoordinate(component, '.selected-metric-line', 'x1')) - .toEqual(component.currentXCoordinate); - expect(getCoordinate(component, '.selected-metric-line', 'x2')) - .toEqual(component.currentXCoordinate); + it('has a line at the currentXCoordinate', () => { + component = createComponent(defaultValuesComponent); + + expect(component.$el.style.left) + .toEqual(`${70 + component.currentXCoordinate}px`); }); - it('has a SVG with the class rect-text-metric at the currentFlagPosition', () => { - const component = createComponent(defaultValuesComponent); + describe('Deployment flag', () => { + it('shows a deployment flag when deployment data provided', () => { + const deploymentFlagComponent = createComponent({ + ...defaultValuesComponent, + deploymentFlagData, + }); + + expect( + deploymentFlagComponent.$el.querySelector('.popover-title'), + ).toContainText('Deployed'); + }); + + it('contains the ref when a tag is available', () => { + const deploymentFlagComponent = createComponent({ + ...defaultValuesComponent, + deploymentFlagData: { + ...deploymentFlagData, + sha: 'f5bcd1d9dac6fa4137e2510b9ccd134ef2e84187', + tag: true, + ref: '1.0', + }, + }); + + expect( + deploymentFlagComponent.$el.querySelector('.deploy-meta-content'), + ).toContainText('f5bcd1d9'); + + expect( + deploymentFlagComponent.$el.querySelector('.deploy-meta-content'), + ).toContainText('1.0'); + }); + + it('does not contain the ref when a tag is unavailable', () => { + const deploymentFlagComponent = createComponent({ + ...defaultValuesComponent, + deploymentFlagData: { + ...deploymentFlagData, + sha: 'f5bcd1d9dac6fa4137e2510b9ccd134ef2e84187', + tag: false, + ref: '1.0', + }, + }); + + expect( + deploymentFlagComponent.$el.querySelector('.deploy-meta-content'), + ).toContainText('f5bcd1d9'); - const svg = component.$el.querySelector('.rect-text-metric'); - expect(svg.tagName).toEqual('svg'); - expect(parseInt(svg.getAttribute('x'), 10)).toEqual(component.currentFlagPosition); + expect( + deploymentFlagComponent.$el.querySelector('.deploy-meta-content'), + ).not.toContainText('1.0'); + }); }); describe('Computed props', () => { - it('calculatedHeight', () => { - const component = createComponent(defaultValuesComponent); + beforeEach(() => { + component = createComponent(defaultValuesComponent); + }); + + it('formatTime', () => { + expect(component.formatTime).toMatch(/\d:17PM/); + }); + + it('formatDate', () => { + expect(component.formatDate).toEqual('Sun, Jun 4'); + }); + + it('cursorStyle', () => { + expect(component.cursorStyle).toEqual({ + top: '20px', + left: '270px', + height: '180px', + }); + }); - expect(component.calculatedHeight).toEqual(180); + it('flagOrientation', () => { + expect(component.flagOrientation).toEqual('left'); }); }); }); -- cgit v1.2.1 From 6762d2875e979e1e2692062b2353265f5d743fc8 Mon Sep 17 00:00:00 2001 From: Brett Walker Date: Mon, 8 Jan 2018 12:44:32 +0000 Subject: Resolve "Allow QA tests to run with `CHROME_HEADLESS=false`" --- qa/qa.rb | 1 + qa/qa/runtime/browser.rb | 41 ++++++++++++++++++++++++----- qa/qa/runtime/env.rb | 15 +++++++++++ qa/spec/runtime/env_spec.rb | 64 +++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 114 insertions(+), 7 deletions(-) create mode 100644 qa/qa/runtime/env.rb create mode 100644 qa/spec/runtime/env_spec.rb diff --git a/qa/qa.rb b/qa/qa.rb index 453e4e9e164..71b80a6adcb 100644 --- a/qa/qa.rb +++ b/qa/qa.rb @@ -10,6 +10,7 @@ module QA autoload :Namespace, 'qa/runtime/namespace' autoload :Scenario, 'qa/runtime/scenario' autoload :Browser, 'qa/runtime/browser' + autoload :Env, 'qa/runtime/env' end ## diff --git a/qa/qa/runtime/browser.rb b/qa/qa/runtime/browser.rb index 220bb45741b..14b2a488760 100644 --- a/qa/qa/runtime/browser.rb +++ b/qa/qa/runtime/browser.rb @@ -38,22 +38,49 @@ module QA Capybara.register_driver :chrome do |app| capabilities = Selenium::WebDriver::Remote::Capabilities.chrome( - 'chromeOptions' => { - 'args' => %w[headless no-sandbox disable-gpu window-size=1280,1680] + # This enables access to logs with `page.driver.manage.get_log(:browser)` + loggingPrefs: { + browser: "ALL", + client: "ALL", + driver: "ALL", + server: "ALL" } ) - Capybara::Selenium::Driver - .new(app, browser: :chrome, desired_capabilities: capabilities) - end + options = Selenium::WebDriver::Chrome::Options.new + options.add_argument("window-size=1240,1680") - Capybara::Screenshot.register_driver(:chrome) do |driver, path| - driver.browser.save_screenshot(path) + # Chrome won't work properly in a Docker container in sandbox mode + options.add_argument("no-sandbox") + + # Run headless by default unless CHROME_HEADLESS is false + if QA::Runtime::Env.chrome_headless? + options.add_argument("headless") + + # Chrome documentation says this flag is needed for now + # https://developers.google.com/web/updates/2017/04/headless-chrome#cli + options.add_argument("disable-gpu") + end + + # Disable /dev/shm use in CI. See https://gitlab.com/gitlab-org/gitlab-ee/issues/4252 + options.add_argument("disable-dev-shm-usage") if QA::Runtime::Env.running_in_ci? + + Capybara::Selenium::Driver.new( + app, + browser: :chrome, + desired_capabilities: capabilities, + options: options + ) end # Keep only the screenshots generated from the last failing test suite Capybara::Screenshot.prune_strategy = :keep_last_run + # From https://github.com/mattheworiordan/capybara-screenshot/issues/84#issuecomment-41219326 + Capybara::Screenshot.register_driver(:chrome) do |driver, path| + driver.browser.save_screenshot(path) + end + Capybara.configure do |config| config.default_driver = :chrome config.javascript_driver = :chrome diff --git a/qa/qa/runtime/env.rb b/qa/qa/runtime/env.rb new file mode 100644 index 00000000000..d5c28e9a7db --- /dev/null +++ b/qa/qa/runtime/env.rb @@ -0,0 +1,15 @@ +module QA + module Runtime + module Env + extend self + + def chrome_headless? + (ENV['CHROME_HEADLESS'] =~ /^(false|no|0)$/i) != 0 + end + + def running_in_ci? + ENV['CI'] || ENV['CI_SERVER'] + end + end + end +end diff --git a/qa/spec/runtime/env_spec.rb b/qa/spec/runtime/env_spec.rb new file mode 100644 index 00000000000..57a72a04507 --- /dev/null +++ b/qa/spec/runtime/env_spec.rb @@ -0,0 +1,64 @@ +describe QA::Runtime::Env do + before do + allow(ENV).to receive(:[]).and_call_original + end + + describe '.chrome_headless?' do + context 'when there is an env variable set' do + it 'returns false when falsey values specified' do + stub_env('CHROME_HEADLESS', 'false') + expect(described_class.chrome_headless?).to be_falsey + + stub_env('CHROME_HEADLESS', 'no') + expect(described_class.chrome_headless?).to be_falsey + + stub_env('CHROME_HEADLESS', '0') + expect(described_class.chrome_headless?).to be_falsey + end + + it 'returns true when anything else specified' do + stub_env('CHROME_HEADLESS', 'true') + expect(described_class.chrome_headless?).to be_truthy + + stub_env('CHROME_HEADLESS', '1') + expect(described_class.chrome_headless?).to be_truthy + + stub_env('CHROME_HEADLESS', 'anything') + expect(described_class.chrome_headless?).to be_truthy + end + end + + context 'when there is no env variable set' do + it 'returns the default, true' do + stub_env('CHROME_HEADLESS', nil) + expect(described_class.chrome_headless?).to be_truthy + end + end + end + + describe '.running_in_ci?' do + context 'when there is an env variable set' do + it 'returns true if CI' do + stub_env('CI', 'anything') + expect(described_class.running_in_ci?).to be_truthy + end + + it 'returns true if CI_SERVER' do + stub_env('CI_SERVER', 'anything') + expect(described_class.running_in_ci?).to be_truthy + end + end + + context 'when there is no env variable set' do + it 'returns true' do + stub_env('CI', nil) + stub_env('CI_SERVER', nil) + expect(described_class.running_in_ci?).to be_falsey + end + end + end + + def stub_env(name, value) + allow(ENV).to receive(:[]).with(name).and_return(value) + end +end -- cgit v1.2.1 From 6732795231dd71f2d5cd8a851372db1894ba0a3f Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Mon, 8 Jan 2018 22:24:23 +0900 Subject: Add memoization for properties --- ...ate_kubernetes_service_to_new_clusters_architectures.rb | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb b/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb index 3fe0a4941d5..11b581e4b57 100644 --- a/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb +++ b/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb @@ -76,19 +76,25 @@ class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migrati end def api_url - JSON.parse(self.properties)['api_url'] + parsed_properties['api_url'] end def ca_pem - JSON.parse(self.properties)['ca_pem'] + parsed_properties['ca_pem'] end def namespace - JSON.parse(self.properties)['namespace'] + parsed_properties['namespace'] end def token - JSON.parse(self.properties)['token'] + parsed_properties['token'] + end + + private + + def parsed_properties + @parsed_properties ||= JSON.parse(self.properties) end end -- cgit v1.2.1 From cb41b5f57cf4c4f29e8a5168572031549ae64e39 Mon Sep 17 00:00:00 2001 From: Luke Bennett Date: Thu, 11 Jan 2018 01:03:52 +0000 Subject: Delete fast_ssh_key_lookup.md (cherry picked from commit 8f8ca9ee3535a6ce50a5574a6353d6cd7e6dbc8f) --- .../operations/fast_ssh_key_lookup.md | 170 --------------------- 1 file changed, 170 deletions(-) delete mode 100644 doc/administration/operations/fast_ssh_key_lookup.md diff --git a/doc/administration/operations/fast_ssh_key_lookup.md b/doc/administration/operations/fast_ssh_key_lookup.md deleted file mode 100644 index 7fe0847facc..00000000000 --- a/doc/administration/operations/fast_ssh_key_lookup.md +++ /dev/null @@ -1,170 +0,0 @@ -# Fast lookup of authorized SSH keys in the database - -Regular SSH operations become slow as the number of users grows because OpenSSH -searches for a key to authorize a user via a linear search. In the worst case, -such as when the user is not authorized to access GitLab, OpenSSH will scan the -entire file to search for a key. This can take significant time and disk I/O, -which will delay users attempting to push or pull to a repository. Making -matters worse, if users add or remove keys frequently, the operating system may -not be able to cache the `authorized_keys` file, which causes the disk to be -accessed repeatedly. - -GitLab Shell solves this by providing a way to authorize SSH users via a fast, -indexed lookup in the GitLab database. This page describes how to enable the fast -lookup of authorized SSH keys. - -> **Warning:** OpenSSH version 6.9+ is required because -`AuthorizedKeysCommand` must be able to accept a fingerprint. These -instructions will break installations using older versions of OpenSSH, such as -those included with CentOS 6 as of September 2017. If you want to use this -feature for CentOS 6, follow [the instructions on how to build and install a custom OpenSSH package](#compiling-a-custom-version-of-openssh-for-centos-6) before continuing. - -## Setting up fast lookup via GitLab Shell - -GitLab Shell provides a way to authorize SSH users via a fast, indexed lookup -to the GitLab database. GitLab Shell uses the fingerprint of the SSH key to -check whether the user is authorized to access GitLab. - -Add the following to your `sshd_config` file. This is usuaully located at -`/etc/ssh/sshd_config`, but it will be `/assets/sshd_config` if you're using -Omnibus Docker: - -``` -AuthorizedKeysCommand /opt/embedded/gitlab-shell/bin/gitlab-shell-authorized-keys-check git %u %k -AuthorizedKeysCommandUser git -``` - -Reload OpenSSH: - -```bash -# Debian or Ubuntu installations -sudo service ssh reload - -# CentOS installations -sudo service sshd reload -``` - -Confirm that SSH is working by removing your user's SSH key in the UI, adding a -new one, and attempting to pull a repo. - -> **Warning:** Do not disable writes until SSH is confirmed to be working -perfectly, because the file will quickly become out-of-date. - -In the case of lookup failures (which are not uncommon), the `authorized_keys` -file will still be scanned. So git SSH performance will still be slow for many -users as long as a large file exists. - -You can disable any more writes to the `authorized_keys` file by unchecking -`Write to "authorized_keys" file` in the Application Settings of your GitLab -installation. - -![Write to authorized keys setting](img/write_to_authorized_keys_setting.png) - -Again, confirm that SSH is working by removing your user's SSH key in the UI, -adding a new one, and attempting to pull a repo. - -Then you can backup and delete your `authorized_keys` file for best performance. - -## How to go back to using the `authorized_keys` file - -This is a brief overview. Please refer to the above instructions for more context. - -1. [Rebuild the `authorized_keys` file](../raketasks/maintenance.md#rebuild-authorized_keys-file) -1. Enable writes to the `authorized_keys` file in Application Settings -1. Remove the `AuthorizedKeysCommand` lines from `/etc/ssh/sshd_config` or from `/assets/sshd_config` if you are using Omnibus Docker. -1. Reload sshd: `sudo service sshd reload` -1. Remove the `/opt/gitlab-shell/authorized_keys` file - -## Compiling a custom version of OpenSSH for CentOS 6 - -Building a custom version of OpenSSH is not necessary for Ubuntu 16.04 users, -since Ubuntu 16.04 ships with OpenSSH 7.2. - -It is also unnecessary for CentOS 7.4 users, as that version ships with -OpenSSH 7.4. If you are using CentOS 7.0 - 7.3, we strongly recommend that you -upgrade to CentOS 7.4 instead of following this procedure. This should be as -simple as running `yum update`. - -CentOS 6 users must build their own OpenSSH package to enable SSH lookups via -the database. The following instructions can be used to build OpenSSH 7.5: - -1. First, download the package and install the required packages: - - ``` - sudo su - - cd /tmp - curl --remote-name https://mirrors.evowise.com/pub/OpenBSD/OpenSSH/portable/openssh-7.5p1.tar.gz - tar xzvf openssh-7.5p1.tar.gz - yum install rpm-build gcc make wget openssl-devel krb5-devel pam-devel libX11-devel xmkmf libXt-devel - ``` - -3. Prepare the build by copying files to the right place: - - ``` - mkdir -p /root/rpmbuild/{SOURCES,SPECS} - cp ./openssh-7.5p1/contrib/redhat/openssh.spec /root/rpmbuild/SPECS/ - cp openssh-7.5p1.tar.gz /root/rpmbuild/SOURCES/ - cd /root/rpmbuild/SPECS - ``` - -3. Next, set the spec settings properly: - - ``` - sed -i -e "s/%define no_gnome_askpass 0/%define no_gnome_askpass 1/g" openssh.spec - sed -i -e "s/%define no_x11_askpass 0/%define no_x11_askpass 1/g" openssh.spec - sed -i -e "s/BuildPreReq/BuildRequires/g" openssh.spec - ``` - -3. Build the RPMs: - - ``` - rpmbuild -bb openssh.spec - ``` - -4. Ensure the RPMs were built: - - ``` - ls -al /root/rpmbuild/RPMS/x86_64/ - ``` - - You should see something as the following: - - ``` - total 1324 - drwxr-xr-x. 2 root root 4096 Jun 20 19:37 . - drwxr-xr-x. 3 root root 19 Jun 20 19:37 .. - -rw-r--r--. 1 root root 470828 Jun 20 19:37 openssh-7.5p1-1.x86_64.rpm - -rw-r--r--. 1 root root 490716 Jun 20 19:37 openssh-clients-7.5p1-1.x86_64.rpm - -rw-r--r--. 1 root root 17020 Jun 20 19:37 openssh-debuginfo-7.5p1-1.x86_64.rpm - -rw-r--r--. 1 root root 367516 Jun 20 19:37 openssh-server-7.5p1-1.x86_64.rpm - ``` - -5. Install the packages. OpenSSH packages will replace `/etc/pam.d/sshd` - with its own version, which may prevent users from logging in, so be sure - that the file is backed up and restored after installation: - - ``` - timestamp=$(date +%s) - cp /etc/pam.d/sshd pam-ssh-conf-$timestamp - rpm -Uvh /root/rpmbuild/RPMS/x86_64/*.rpm - yes | cp pam-ssh-conf-$timestamp /etc/pam.d/sshd - ``` - -6. Verify the installed version. In another window, attempt to login to the server: - - ``` - ssh -v - ``` - - You should see a line that reads: "debug1: Remote protocol version 2.0, remote software version OpenSSH_7.5" - - If not, you may need to restart sshd (e.g. `systemctl restart sshd.service`). - -7. *IMPORTANT!* Open a new SSH session to your server before exiting to make - sure everything is working! If you need to downgrade, simple install the - older package: - - ``` - # Only run this if you run into a problem logging in - yum downgrade openssh-server openssh openssh-clients - ``` \ No newline at end of file -- cgit v1.2.1 From 5c4e6e814effb1062a054880d6a224c5da814346 Mon Sep 17 00:00:00 2001 From: Luke Bennett Date: Thu, 11 Jan 2018 13:47:36 +0000 Subject: Revert "Merge branch '40418-migrate-existing-data-from-kubernetesservice-to-clusters-platforms-kubernetes' into 'master'" This reverts commit 4b92efd90cedaa0aff218d11fdce279701128bea, reversing changes made to 33cea50976a6a066641b696527dc2d9e980183fb. --- ...tesservice-to-clusters-platforms-kubernetes.yml | 5 - ...rnetes_service_to_new_clusters_architectures.rb | 151 ---------- ...s_service_to_new_clusters_architectures_spec.rb | 312 --------------------- 3 files changed, 468 deletions(-) delete mode 100644 changelogs/unreleased/40418-migrate-existing-data-from-kubernetesservice-to-clusters-platforms-kubernetes.yml delete mode 100644 db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb delete mode 100644 spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb diff --git a/changelogs/unreleased/40418-migrate-existing-data-from-kubernetesservice-to-clusters-platforms-kubernetes.yml b/changelogs/unreleased/40418-migrate-existing-data-from-kubernetesservice-to-clusters-platforms-kubernetes.yml deleted file mode 100644 index 5e158d831a6..00000000000 --- a/changelogs/unreleased/40418-migrate-existing-data-from-kubernetesservice-to-clusters-platforms-kubernetes.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Migrate existing data from KubernetesService to Clusters::Platforms::Kubernetes -merge_request: 15589 -author: -type: changed diff --git a/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb b/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb deleted file mode 100644 index 11b581e4b57..00000000000 --- a/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb +++ /dev/null @@ -1,151 +0,0 @@ -class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migration - include Gitlab::Database::MigrationHelpers - - DOWNTIME = false - DEFAULT_KUBERNETES_SERVICE_CLUSTER_NAME = 'KubernetesService'.freeze - - disable_ddl_transaction! - - class Project < ActiveRecord::Base - self.table_name = 'projects' - - has_many :cluster_projects, class_name: 'MigrateKubernetesServiceToNewClustersArchitectures::ClustersProject' - has_many :clusters, through: :cluster_projects, class_name: 'MigrateKubernetesServiceToNewClustersArchitectures::Cluster' - has_many :services, class_name: 'MigrateKubernetesServiceToNewClustersArchitectures::Service' - has_one :kubernetes_service, -> { where(category: 'deployment', type: 'KubernetesService') }, class_name: 'MigrateKubernetesServiceToNewClustersArchitectures::Service', inverse_of: :project, foreign_key: :project_id - end - - class Cluster < ActiveRecord::Base - self.table_name = 'clusters' - - has_many :cluster_projects, class_name: 'MigrateKubernetesServiceToNewClustersArchitectures::ClustersProject' - has_many :projects, through: :cluster_projects, class_name: 'MigrateKubernetesServiceToNewClustersArchitectures::Project' - has_one :platform_kubernetes, class_name: 'MigrateKubernetesServiceToNewClustersArchitectures::PlatformsKubernetes' - - accepts_nested_attributes_for :platform_kubernetes - - enum platform_type: { - kubernetes: 1 - } - - enum provider_type: { - user: 0, - gcp: 1 - } - end - - class ClustersProject < ActiveRecord::Base - self.table_name = 'cluster_projects' - - belongs_to :cluster, class_name: 'MigrateKubernetesServiceToNewClustersArchitectures::Cluster' - belongs_to :project, class_name: 'MigrateKubernetesServiceToNewClustersArchitectures::Project' - end - - class PlatformsKubernetes < ActiveRecord::Base - self.table_name = 'cluster_platforms_kubernetes' - - belongs_to :cluster, class_name: 'MigrateKubernetesServiceToNewClustersArchitectures::Cluster' - - attr_encrypted :token, - mode: :per_attribute_iv, - key: Gitlab::Application.secrets.db_key_base, - algorithm: 'aes-256-cbc' - end - - class Service < ActiveRecord::Base - include EachBatch - - self.table_name = 'services' - self.inheritance_column = :_type_disabled # Disable STI, otherwise KubernetesModel will be looked up - - belongs_to :project, class_name: 'MigrateKubernetesServiceToNewClustersArchitectures::Project', foreign_key: :project_id - - scope :unmanaged_kubernetes_service, -> do - joins('LEFT JOIN projects ON projects.id = services.project_id') - .joins('LEFT JOIN cluster_projects ON cluster_projects.project_id = projects.id') - .joins('LEFT JOIN cluster_platforms_kubernetes ON cluster_platforms_kubernetes.cluster_id = cluster_projects.cluster_id') - .where(category: 'deployment', type: 'KubernetesService', template: false) - .where("services.properties LIKE '%api_url%'") - .where("(services.properties NOT LIKE CONCAT('%', cluster_platforms_kubernetes.api_url, '%')) OR cluster_platforms_kubernetes.api_url IS NULL") - .group(:id) - .order(id: :asc) - end - - scope :kubernetes_service_without_template, -> do - where(category: 'deployment', type: 'KubernetesService', template: false) - end - - def api_url - parsed_properties['api_url'] - end - - def ca_pem - parsed_properties['ca_pem'] - end - - def namespace - parsed_properties['namespace'] - end - - def token - parsed_properties['token'] - end - - private - - def parsed_properties - @parsed_properties ||= JSON.parse(self.properties) - end - end - - def find_dedicated_environement_scope(project) - environment_scopes = project.clusters.map(&:environment_scope) - - return '*' if environment_scopes.exclude?('*') # KubernetesService should be added as a default cluster (environment_scope: '*') at first place - return 'migrated/*' if environment_scopes.exclude?('migrated/*') # If it's conflicted, the KubernetesService added as a migrated cluster - - unique_iid = 0 - - # If it's still conflicted, finding an unique environment scope incrementaly - loop do - candidate = "migrated#{unique_iid}/*" - return candidate if environment_scopes.exclude?(candidate) - - unique_iid += 1 - end - end - - def up - ActiveRecord::Base.transaction do - MigrateKubernetesServiceToNewClustersArchitectures::Service - .unmanaged_kubernetes_service.find_each(batch_size: 1) do |kubernetes_service| - MigrateKubernetesServiceToNewClustersArchitectures::Cluster.create( - enabled: kubernetes_service.active, - user_id: nil, # KubernetesService doesn't have - name: DEFAULT_KUBERNETES_SERVICE_CLUSTER_NAME, - provider_type: MigrateKubernetesServiceToNewClustersArchitectures::Cluster.provider_types[:user], - platform_type: MigrateKubernetesServiceToNewClustersArchitectures::Cluster.platform_types[:kubernetes], - projects: [kubernetes_service.project], - environment_scope: find_dedicated_environement_scope(kubernetes_service.project), - platform_kubernetes_attributes: { - api_url: kubernetes_service.api_url, - ca_cert: kubernetes_service.ca_pem, - namespace: kubernetes_service.namespace, - username: nil, # KubernetesService doesn't have - encrypted_password: nil, # KubernetesService doesn't have - encrypted_password_iv: nil, # KubernetesService doesn't have - token: kubernetes_service.token # encrypted_token and encrypted_token_iv - } ) - end - end - - MigrateKubernetesServiceToNewClustersArchitectures::Service - .kubernetes_service_without_template.each_batch(of: 100) do |kubernetes_service| - kubernetes_service.update_all(active: false) - end - end - - def down - # noop - end -end diff --git a/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb b/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb deleted file mode 100644 index df0015b6dd3..00000000000 --- a/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb +++ /dev/null @@ -1,312 +0,0 @@ -require 'spec_helper' -require Rails.root.join('db', 'post_migrate', '20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb') - -describe MigrateKubernetesServiceToNewClustersArchitectures, :migration do - context 'when unique KubernetesService exists' do - shared_examples 'KubernetesService migration' do - let(:sample_num) { 2 } - - let(:projects) do - (1..sample_num).each_with_object([]) do |n, array| - array << MigrateKubernetesServiceToNewClustersArchitectures::Project.create! - end - end - - let!(:kubernetes_services) do - projects.map do |project| - MigrateKubernetesServiceToNewClustersArchitectures::Service.create!( - project: project, - active: active, - category: 'deployment', - type: 'KubernetesService', - properties: "{\"namespace\":\"prod\",\"api_url\":\"https://kubernetes#{project.id}.com\",\"ca_pem\":\"ca_pem#{project.id}\",\"token\":\"token#{project.id}\"}") - end - end - - it 'migrates the KubernetesService to Platform::Kubernetes' do - expect { migrate! }.to change { MigrateKubernetesServiceToNewClustersArchitectures::Cluster.count }.by(sample_num) - - projects.each do |project| - project.clusters.last.tap do |cluster| - expect(cluster.enabled).to eq(active) - expect(cluster.platform_kubernetes.api_url).to eq(project.kubernetes_service.api_url) - expect(cluster.platform_kubernetes.ca_cert).to eq(project.kubernetes_service.ca_pem) - expect(cluster.platform_kubernetes.token).to eq(project.kubernetes_service.token) - expect(project.kubernetes_service).not_to be_active - end - end - end - end - - context 'when KubernetesService is active' do - let(:active) { true } - - it_behaves_like 'KubernetesService migration' - end - end - - context 'when unique KubernetesService spawned from Service Template' do - let(:sample_num) { 2 } - - let(:projects) do - (1..sample_num).each_with_object([]) do |n, array| - array << MigrateKubernetesServiceToNewClustersArchitectures::Project.create! - end - end - - let!(:kubernetes_service_template) do - MigrateKubernetesServiceToNewClustersArchitectures::Service.create!( - template: true, - category: 'deployment', - type: 'KubernetesService', - properties: "{\"namespace\":\"prod\",\"api_url\":\"https://sample.kubernetes.com\",\"ca_pem\":\"ca_pem-sample\",\"token\":\"token-sample\"}") - end - - let!(:kubernetes_services) do - projects.map do |project| - MigrateKubernetesServiceToNewClustersArchitectures::Service.create!( - project: project, - category: 'deployment', - type: 'KubernetesService', - properties: "{\"namespace\":\"prod\",\"api_url\":\"#{kubernetes_service_template.api_url}\",\"ca_pem\":\"#{kubernetes_service_template.ca_pem}\",\"token\":\"#{kubernetes_service_template.token}\"}") - end - end - - it 'migrates the KubernetesService to Platform::Kubernetes without template' do - expect { migrate! }.to change { MigrateKubernetesServiceToNewClustersArchitectures::Cluster.count }.by(sample_num) - - projects.each do |project| - project.clusters.last.tap do |cluster| - expect(cluster.platform_kubernetes.api_url).to eq(project.kubernetes_service.api_url) - expect(cluster.platform_kubernetes.ca_cert).to eq(project.kubernetes_service.ca_pem) - expect(cluster.platform_kubernetes.token).to eq(project.kubernetes_service.token) - expect(project.kubernetes_service).not_to be_active - end - end - end - end - - context 'when managed KubernetesService exists' do - let(:project) { MigrateKubernetesServiceToNewClustersArchitectures::Project.create! } - - let(:cluster) do - MigrateKubernetesServiceToNewClustersArchitectures::Cluster.create!( - projects: [project], - name: 'sample-cluster', - platform_type: :kubernetes, - provider_type: :user, - platform_kubernetes_attributes: { - api_url: 'https://sample.kubernetes.com', - ca_cert: 'ca_pem-sample', - token: 'token-sample' - } ) - end - - let!(:kubernetes_service) do - MigrateKubernetesServiceToNewClustersArchitectures::Service.create!( - project: project, - active: cluster.enabled, - category: 'deployment', - type: 'KubernetesService', - properties: "{\"api_url\":\"#{cluster.platform_kubernetes.api_url}\"}") - end - - it 'does not migrate the KubernetesService and disables the kubernetes_service' do # Because the corresponding Platform::Kubernetes already exists - expect { migrate! }.not_to change { MigrateKubernetesServiceToNewClustersArchitectures::Cluster.count } - - kubernetes_service.reload - expect(kubernetes_service).not_to be_active - end - end - - context 'when production cluster has already been existed' do # i.e. There are no environment_scope conflicts - let(:project) { MigrateKubernetesServiceToNewClustersArchitectures::Project.create! } - - let(:cluster) do - MigrateKubernetesServiceToNewClustersArchitectures::Cluster.create!( - projects: [project], - name: 'sample-cluster', - platform_type: :kubernetes, - provider_type: :user, - environment_scope: 'production/*', - platform_kubernetes_attributes: { - api_url: 'https://sample.kubernetes.com', - ca_cert: 'ca_pem-sample', - token: 'token-sample' - } ) - end - - let!(:kubernetes_service) do - MigrateKubernetesServiceToNewClustersArchitectures::Service.create!( - project: project, - active: true, - category: 'deployment', - type: 'KubernetesService', - properties: "{\"api_url\":\"https://debug.kube.com\"}") - end - - it 'migrates the KubernetesService to Platform::Kubernetes' do - expect { migrate! }.to change { MigrateKubernetesServiceToNewClustersArchitectures::Cluster.count }.by(1) - - kubernetes_service.reload - project.clusters.last.tap do |cluster| - expect(cluster.environment_scope).to eq('*') - expect(cluster.platform_kubernetes.api_url).to eq(kubernetes_service.api_url) - expect(cluster.platform_kubernetes.ca_cert).to eq(kubernetes_service.ca_pem) - expect(cluster.platform_kubernetes.token).to eq(kubernetes_service.token) - expect(kubernetes_service).not_to be_active - end - end - end - - context 'when default cluster has already been existed' do - let(:project) { MigrateKubernetesServiceToNewClustersArchitectures::Project.create! } - - let!(:cluster) do - MigrateKubernetesServiceToNewClustersArchitectures::Cluster.create!( - projects: [project], - name: 'sample-cluster', - platform_type: :kubernetes, - provider_type: :user, - environment_scope: '*', - platform_kubernetes_attributes: { - api_url: 'https://sample.kubernetes.com', - ca_cert: 'ca_pem-sample', - token: 'token-sample' - } ) - end - - let!(:kubernetes_service) do - MigrateKubernetesServiceToNewClustersArchitectures::Service.create!( - project: project, - active: true, - category: 'deployment', - type: 'KubernetesService', - properties: "{\"api_url\":\"https://debug.kube.com\"}") - end - - it 'migrates the KubernetesService to Platform::Kubernetes with dedicated environment_scope' do # Because environment_scope is duplicated - expect { migrate! }.to change { MigrateKubernetesServiceToNewClustersArchitectures::Cluster.count }.by(1) - - kubernetes_service.reload - project.clusters.last.tap do |cluster| - expect(cluster.environment_scope).to eq('migrated/*') - expect(cluster.platform_kubernetes.api_url).to eq(kubernetes_service.api_url) - expect(cluster.platform_kubernetes.ca_cert).to eq(kubernetes_service.ca_pem) - expect(cluster.platform_kubernetes.token).to eq(kubernetes_service.token) - expect(kubernetes_service).not_to be_active - end - end - end - - context 'when default cluster and migrated cluster has already been existed' do - let(:project) { MigrateKubernetesServiceToNewClustersArchitectures::Project.create! } - - let!(:cluster) do - MigrateKubernetesServiceToNewClustersArchitectures::Cluster.create!( - projects: [project], - name: 'sample-cluster', - platform_type: :kubernetes, - provider_type: :user, - environment_scope: '*', - platform_kubernetes_attributes: { - api_url: 'https://sample.kubernetes.com', - ca_cert: 'ca_pem-sample', - token: 'token-sample' - } ) - end - - let!(:migrated_cluster) do - MigrateKubernetesServiceToNewClustersArchitectures::Cluster.create!( - projects: [project], - name: 'sample-cluster', - platform_type: :kubernetes, - provider_type: :user, - environment_scope: 'migrated/*', - platform_kubernetes_attributes: { - api_url: 'https://sample.kubernetes.com', - ca_cert: 'ca_pem-sample', - token: 'token-sample' - } ) - end - - let!(:kubernetes_service) do - MigrateKubernetesServiceToNewClustersArchitectures::Service.create!( - project: project, - active: true, - category: 'deployment', - type: 'KubernetesService', - properties: "{\"api_url\":\"https://debug.kube.com\"}") - end - - it 'migrates the KubernetesService to Platform::Kubernetes with dedicated environment_scope' do # Because environment_scope is duplicated - expect { migrate! }.to change { MigrateKubernetesServiceToNewClustersArchitectures::Cluster.count }.by(1) - - kubernetes_service.reload - project.clusters.last.tap do |cluster| - expect(cluster.environment_scope).to eq('migrated0/*') - expect(cluster.platform_kubernetes.api_url).to eq(kubernetes_service.api_url) - expect(cluster.platform_kubernetes.ca_cert).to eq(kubernetes_service.ca_pem) - expect(cluster.platform_kubernetes.token).to eq(kubernetes_service.token) - expect(kubernetes_service).not_to be_active - end - end - end - - context 'when KubernetesService has nullified parameters' do - let(:project) { MigrateKubernetesServiceToNewClustersArchitectures::Project.create! } - - before do - MigrateKubernetesServiceToNewClustersArchitectures::Service.create!( - project: project, - active: false, - category: 'deployment', - type: 'KubernetesService', - properties: "{}") - end - - it 'does not migrate the KubernetesService and disables the kubernetes_service' do - expect { migrate! }.not_to change { MigrateKubernetesServiceToNewClustersArchitectures::Cluster.count } - - expect(project.kubernetes_service).not_to be_active - end - end - - # Platforms::Kubernetes validates `token` reagdless of the activeness, - # whereas KubernetesService validates `token` if only it's activated - # However, in this migration file, there are no validations because of the re-defined model class - # therefore, we should safely add this raw to Platform::Kubernetes - context 'when KubernetesService has empty token' do - let(:project) { MigrateKubernetesServiceToNewClustersArchitectures::Project.create! } - - before do - MigrateKubernetesServiceToNewClustersArchitectures::Service.create!( - project: project, - active: false, - category: 'deployment', - type: 'KubernetesService', - properties: "{\"namespace\":\"prod\",\"api_url\":\"http://111.111.111.111\",\"ca_pem\":\"a\",\"token\":\"\"}") - end - - it 'does not migrate the KubernetesService and disables the kubernetes_service' do - expect { migrate! }.to change { MigrateKubernetesServiceToNewClustersArchitectures::Cluster.count }.by(1) - - project.clusters.last.tap do |cluster| - expect(cluster.environment_scope).to eq('*') - expect(cluster.platform_kubernetes.namespace).to eq('prod') - expect(cluster.platform_kubernetes.api_url).to eq('http://111.111.111.111') - expect(cluster.platform_kubernetes.ca_cert).to eq('a') - expect(cluster.platform_kubernetes.token).to be_empty - expect(project.kubernetes_service).not_to be_active - end - end - end - - context 'when KubernetesService does not exist' do - let!(:project) { MigrateKubernetesServiceToNewClustersArchitectures::Project.create! } - - it 'does not migrate the KubernetesService' do - expect { migrate! }.not_to change { MigrateKubernetesServiceToNewClustersArchitectures::Cluster.count } - end - end -end -- cgit v1.2.1