Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

DO-494: Refactor Azure to use resource groups #889

Merged
merged 5 commits into from
Jan 20, 2023
Merged

DO-494: Refactor Azure to use resource groups #889

merged 5 commits into from
Jan 20, 2023

Conversation

atodorov
Copy link
Collaborator

Description of proposed changes:

Practical tips for PR review & merge:

  • All GitHub Actions report PASS
  • Newly added code/functions have unit tests
    • Coverage tools report all newly added lines as covered
    • The positive scenario is exercised
    • Negative scenarios are exercised, e.g. assert on all possible errors
    • Assert on events triggered if applicable
    • Assert on changes made to storage if applicable
  • Modified behavior/functions - try to make sure above test items are covered
  • Integration tests are added if applicable/needed

@atodorov atodorov changed the base branch from dev to testnet January 18, 2023 18:38
@atodorov atodorov force-pushed the ci/DO-494-refactor-azure-to-use-resource-groups branch 2 times, most recently from b133954 to 9e6992b Compare January 18, 2023 20:11
@github-actions
Copy link

For full LLVM coverage report click here!

@atodorov atodorov force-pushed the ci/DO-494-refactor-azure-to-use-resource-groups branch from 34bbf87 to dfdbf09 Compare January 18, 2023 20:53
@codecov-commenter
Copy link

Codecov Report

Merging #889 (34bbf87) into testnet (54e9f6c) will decrease coverage by 1.85%.
The diff coverage is 82.73%.

❗ Current head 34bbf87 differs from pull request most recent head dfdbf09. Consider uploading reports for the commit dfdbf09 to get more accurate results

@@             Coverage Diff             @@
##           testnet     #889      +/-   ##
===========================================
- Coverage    79.97%   78.12%   -1.85%     
===========================================
  Files           50       68      +18     
  Lines        10056    10766     +710     
===========================================
+ Hits          8042     8411     +369     
- Misses        2014     2355     +341
Impacted Files Coverage Δ
node/src/benchmarking.rs 0.00% <0.00%> (ø)
node/src/chain_spec.rs 0.00% <0.00%> (ø)
node/src/cli.rs 35.71% <0.00%> (+1.16%) ⬆️
node/src/command.rs 0.00% <0.00%> (ø)
node/src/main.rs 16.66% <ø> (ø)
node/src/rpc.rs 0.00% <0.00%> (ø)
node/src/service.rs 0.00% <0.00%> (ø)
node/src/service/nonce_monitor.rs 0.00% <0.00%> (ø)
pallets/creditcoin/src/migrations/mod.rs 0.00% <0.00%> (ø)
pallets/creditcoin/src/ocw.rs 96.00% <ø> (-0.50%) ⬇️
... and 89 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

notaturkey and others added 5 commits January 18, 2023 22:06
@notaturkey @atodorov
Create/remove a dedicated Azure resource group for GH runners
b5b2b30 
- remove automatically loading SSH keys insid runner.bicep
- authorized_keys now need to be passed as a parameter

- adjust formatting in yaml file
@atodorov
Execute Azure RG/VM removal if previous job wasn't skipped
78ad4b2 
this prevents failures when trying to remove non-existing resource
groups. Usually happens for automatic commits b/c we don't execute the
benchmarks for them. See
https://github.com/gluwa/creditcoin/actions/runs/3952337176/jobs/6767291602
@atodorov
Remove unnecessary outputs for remove-github-runner job
e1f7a77
@atodorov
Use env.AZ_LOCATION directly
7a861bc 
it's a hard-coded value and we don't need to interpret the value in
intermediate steps like we do for RUNNER_VM_NAME.
@atodorov
Separate RUN_ID and RUN_ATTEMPT numbers in the names
3b37d42 
for better readability
@atodorov atodorov changed the base branch from testnet to dev January 18, 2023 22:06
@atodorov atodorov force-pushed the ci/DO-494-refactor-azure-to-use-resource-groups branch from 0f4dd46 to 3b37d42 Compare January 18, 2023 22:06
@atodorov
Copy link
Collaborator Author

Running benchmarks for regular commit - https://github.com/gluwa/creditcoin/actions/runs/3952834632/jobs/6768400236 - PASS
All jobs skipped, no failure for Auto-update weights commit - https://github.com/gluwa/creditcoin/actions/runs/3953050499/jobs/6768869236

This is now ready for review.

@atodorov atodorov marked this pull request as ready for review January 18, 2023 22:08
@atodorov atodorov mentioned this pull request Jan 19, 2023
Merged
9 tasks
notaturkey
notaturkey approved these changes Jan 19, 2023
View reviewed changes
Copy link
Contributor

@notaturkey notaturkey left a comment

Choose a reason for hiding this comment

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

tested and good to merge

@atodorov atodorov merged commit deda03d into dev Jan 20, 2023
@atodorov atodorov deleted the ci/DO-494-refactor-azure-to-use-resource-groups branch January 20, 2023 13:19
@atodorov atodorov mentioned this pull request Jan 23, 2023
Closed
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@notaturkey notaturkey notaturkey approved these changes

@CAGS295 CAGS295 CAGS295 approved these changes

@AdaJane AdaJane Awaiting requested review from AdaJane

@nathanwhit nathanwhit Awaiting requested review from nathanwhit

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@atodorov @codecov-commenter @notaturkey @CAGS295