Skip to content

MSF-21292: Add capability to remove old master workspaces to LCM brick#1802

Merged
1 commit merged intogooddata:masterfrom
dangdinh:MSF-21292
Oct 4, 2021
Merged

MSF-21292: Add capability to remove old master workspaces to LCM brick#1802
1 commit merged intogooddata:masterfrom
dangdinh:MSF-21292

Conversation

@dangdinh
Copy link
Copy Markdown

@dangdinh dangdinh commented Sep 8, 2021

  • Add capability to remove old master workspaces to LCM Rollout brick.
  • Verified ADS and Netapp configuration on PI

@ghost
Copy link
Copy Markdown

ghost commented Sep 8, 2021

@ghost
Copy link
Copy Markdown

ghost commented Sep 8, 2021

Build in pipeline check aborted.

1 similar comment
@ghost
Copy link
Copy Markdown

ghost commented Sep 8, 2021

Build in pipeline check aborted.

@ghost
Copy link
Copy Markdown

ghost commented Sep 8, 2021

Build in pipeline check aborted.

@ghost
Copy link
Copy Markdown

ghost commented Sep 8, 2021

@ghost
Copy link
Copy Markdown

ghost commented Sep 8, 2021

Build in pipeline check aborted.

@ghost
Copy link
Copy Markdown

ghost commented Sep 8, 2021

@ghost
Copy link
Copy Markdown

ghost commented Sep 10, 2021

Build in pipeline check aborted.

@ghost
Copy link
Copy Markdown

ghost commented Sep 10, 2021

project = client.projects(project_wrapper[:master_project_id])
GoodData.logger.info "Segment #{segment.segment_id}: Deleting old master workspace, project: '#{project.title}', PID: (#{project.pid})."
project.delete
removal_master_project_ids << project_wrapper[:master_project_id]
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add catch exception for each project deletion.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated it

@ghost
Copy link
Copy Markdown

ghost commented Sep 14, 2021

Build in pipeline check aborted.

@ghost
Copy link
Copy Markdown

ghost commented Sep 14, 2021

@ghost
Copy link
Copy Markdown

ghost commented Sep 14, 2021

Build in pipeline check aborted.

@ghost
Copy link
Copy Markdown

ghost commented Sep 14, 2021

@danh-ung danh-ung added the do not merge Do not merge this yet label Sep 14, 2021
@ghost
Copy link
Copy Markdown

ghost commented Sep 16, 2021

Build in pipeline check aborted.

@ghost
Copy link
Copy Markdown

ghost commented Sep 16, 2021

begin
removal_master_project_ids = remove_old_master_workspace(params, segment.segment_id, master_projects, number_of_deleted_projects)
remove_release_table(params, domain_name, data_product.data_product_id, segment.segment_id, master_projects, removal_master_project_ids)
rescue => e
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rescue StandardError => e , Or we can move rescue to remove_release_table function, we already have rescue for remove_old_master_workspace

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated it

"Details: #{sync_result['links']['details']}")
end

# rubocop:disable Style/RescueStandardError
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove this

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated it

end
end
end
# rubocop:enable Style/RescueStandardError
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And remove this

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated it

[
{ master_project_id: 'foo', version: 2, segment_id: 'premium' },
{ master_project_id: 'bar', version: 3, segment_id: 'premium' },
{ master_project_id: 'baz', version: 1, segment_id: 'premium' }
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please add test data with: 9, 12, 113

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated it

expect(GoodData::LCM2::Helpers.get_master_project_list_from_nfs(domain_id, data_prod_id, 'first_segment')).to eq(result)
end
end
end
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add test for update_master_project_from_nfs and remove_old_master_workspace

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated it

GoodData.logger.info "Segment #{segment_id}: Deleting old master workspace, project: '#{project.title}', PID: (#{project.pid})."
project.delete if project && !project.deleted?
removal_master_project_ids << project_wrapper[:master_project_id]
rescue => ex
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also rescue StandardError => ex

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated it

if keep_only_previous_masters_count >= 0
number_of_deleted_projects = master_projects.count - (keep_only_previous_masters_count + 1)

if number_of_deleted_projects.positive?
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why don't we just simply check something like: master_projects.count - keep_only_previous_masters_count > 1

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

discussed about it and we keep it as current because we need to remove latest master workspace from list

project.delete if project && !project.deleted?
removal_master_project_ids << project_wrapper[:master_project_id]
rescue => ex
GoodData.logger.error "Unable to remove master workspace, reason: #{ex}"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Unable to remove master workspace: '#{master_project_id}', Error: #{ex}"

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated it

@ghost
Copy link
Copy Markdown

ghost commented Sep 20, 2021

Build in pipeline check aborted.

1 similar comment
@ghost
Copy link
Copy Markdown

ghost commented Sep 20, 2021

Build in pipeline check aborted.

@ghost
Copy link
Copy Markdown

ghost commented Sep 20, 2021

Build in pipeline check aborted.

@ghost
Copy link
Copy Markdown

ghost commented Sep 22, 2021

Build in pipeline check aborted.

1 similar comment
@ghost
Copy link
Copy Markdown

ghost commented Sep 22, 2021

Build in pipeline check aborted.

@ghost
Copy link
Copy Markdown

ghost commented Sep 22, 2021

Build in pipeline check aborted.

1 similar comment
@ghost
Copy link
Copy Markdown

ghost commented Sep 22, 2021

Build in pipeline check aborted.

@ghost
Copy link
Copy Markdown

ghost commented Sep 22, 2021

Build in pipeline check aborted.

@ghost
Copy link
Copy Markdown

ghost commented Sep 22, 2021

@ghost
Copy link
Copy Markdown

ghost commented Sep 24, 2021

Build in pipeline check aborted.

1 similar comment
@ghost
Copy link
Copy Markdown

ghost commented Sep 24, 2021

Build in pipeline check aborted.

@ghost
Copy link
Copy Markdown

ghost commented Sep 24, 2021

Build in pipeline check aborted.

@ghost
Copy link
Copy Markdown

ghost commented Sep 24, 2021

@danh-ung danh-ung removed the do not merge Do not merge this yet label Sep 25, 2021
@danh-ung
Copy link
Copy Markdown

LGTM

let(:latest_data) do
[
{ master_project_id: 'opi15', version: 3 }
]
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here we test only WRITE "one master project", missing usecase WRITE "many master projects". It can miss the bug that our loop to write multiple projects doesn't work.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated it

@ghost
Copy link
Copy Markdown

ghost commented Sep 28, 2021

Build in pipeline check aborted.

@ghost
Copy link
Copy Markdown

ghost commented Sep 28, 2021

@ghost
Copy link
Copy Markdown

ghost commented Sep 28, 2021

Build in pipeline check aborted.

@ghost
Copy link
Copy Markdown

ghost commented Sep 28, 2021

@sangtm sangtm added the merge label Oct 4, 2021
@yenkins
Copy link
Copy Markdown

yenkins commented Oct 4, 2021

Sonar scan result

More detail, see in https://sonarqube-gate.intgdc.com/dashboard?id=gooddata-ruby-gate-PR1802

To scan for vulnerabilities in dependencies and run unit tests (to get coverage report in sonar) please comment your PR with 'extended check sonar'.

@ghost ghost removed the merge label Oct 4, 2021
@ghost ghost merged commit ac4cb7b into gooddata:master Oct 4, 2021
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants