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.

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

chore: move lxd related files to lxd_cloud module #254

Closed
wants to merge 3 commits into from

Conversation

yanksyoon
Copy link
Collaborator

Applicable spec: N/A

Overview

This moves all lxd related files to new lxd_cloud module.

Rationale

To prepare for refactor which caters multiple cloud types.

Juju Events Changes

None.

Module Changes

All LXD related files have been moved to lxd_cloud module.

Library Changes

None.

Checklist

Copy link
Contributor

github-actions bot commented Apr 8, 2024

Test coverage for 04a120f

Name                                             Stmts   Miss Branch BrPart  Cover   Missing
--------------------------------------------------------------------------------------------
src/charm.py                                       473    109    107     21    75%   100-101, 103-104, 109-110, 177-179, 225-244, 259-261, 262->266, 289-293, 370-396, 429-434, 448, 469, 481-487, 501-502, 515-541, 572-573, 576->581, 615, 619-624, 664, 673->676, 690-691, 723-736, 765-770, 792, 836-837, 839-840, 842-843, 917->919, 984-985, 1018-1020, 1037
src/charm_state.py                                 291     33     64     10    87%   100, 246, 272, 276-277, 297-299, 307-308, 336-337, 374-375, 381-382, 409, 414, 416, 420, 459, 471-478, 638-639, 644-645, 651-656
src/errors.py                                       42      0      0      0   100%
src/github_client.py                                93     16     40      5    78%   39-46, 94->exit, 99-100, 167, 190, 203-210, 228->263, 257
src/github_metrics.py                               14      0      0      0   100%
src/github_type.py                                  50      0      0      0   100%
src/lxd_cloud/__init__.py                            1      0      0      0   100%
src/lxd_cloud/event_timer.py                        55      7      2      1    86%   103-104, 127, 144-145, 161-162
src/lxd_cloud/firewall.py                           52     18     20      0    61%   42-43, 66-69, 110-184
src/lxd_cloud/lxd.py                               177    118     16      0    31%   47, 55, 68-84, 101-108, 121-134, 150-153, 172-174, 183, 197-201, 215-219, 231-235, 259-266, 278, 289-293, 309-314, 326, 340-344, 359-363, 374-378, 393-399, 403-404, 408, 421, 432-433, 452, 460, 471-475, 486, 497, 521-527, 531-532, 536, 548, 560-562, 571-576
src/lxd_cloud/lxd_type.py                           37      0      2      0   100%
src/lxd_cloud/metrics.py                            73      2     10      1    96%   61->64, 170-171
src/lxd_cloud/repo_policy_compliance_client.py      19     11      0      0    42%   31-33, 41-48
src/lxd_cloud/runner.py                            324     71     96     24    73%   46->48, 176->190, 230-231, 257-258, 299-308, 332-337, 342, 362, 366-376, 425->428, 431-433, 440, 454, 464, 468, 470, 485, 519-524, 540-553, 564-603, 608, 650, 703-705, 709, 727, 762, 788, 793-805, 819, 824->826, 829-831
src/lxd_cloud/runner_logs.py                        35      2      6      1    93%   62->61, 66-67
src/lxd_cloud/runner_manager.py                    309     56    110     13    81%   122, 164-166, 179-180, 192-194, 200-205, 209-210, 240, 283-284, 328-330, 404, 414, 440, 451-455, 480, 532-535, 567-585, 598-603, 627-629, 647-648, 746
src/lxd_cloud/runner_manager_type.py                39      0      6      0   100%
src/lxd_cloud/runner_metrics.py                    122      8     20      2    93%   146-147, 159, 190-191, 306-310
src/lxd_cloud/shared_fs.py                         116     16     20      0    88%   108-109, 133-134, 217-218, 241-242, 254-255, 260-261, 267-268, 292-293
src/metrics_type.py                                  6      0      0      0   100%
src/openstack_cloud/__init__.py                     22      0      2      0   100%
src/openstack_cloud/openstack_manager.py           112     13     22      0    89%   303-306, 353, 379-401
src/runner_type.py                                  32      0      8      0   100%
src/utilities.py                                    68      7     20      7    82%   74->76, 78->84, 91, 121, 135, 174-177, 228
--------------------------------------------------------------------------------------------
TOTAL                                             2562    487    571     85    79%

Static code analysis report

Run started:2024-04-08 10:20:20.432792

Test results:
  No issues identified.

Code scanned:
  Total lines of code: 5282
  Total lines skipped (#nosec): 0
  Total potential issues skipped due to specifically being disabled (e.g., #nosec BXXX): 7

Run metrics:
  Total issues (by severity):
  	Undefined: 0
  	Low: 0
  	Medium: 0
  	High: 0
  Total issues (by confidence):
  	Undefined: 0
  	Low: 0
  	Medium: 0
  	High: 0
Files skipped (0):

Copy link
Collaborator

@cbartz cbartz left a comment

Choose a reason for hiding this comment

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

One comment, otherwise it looks fine to me. I assume that some of the functionality now placed in lxd_cloud will be extracted into more common modules in the future (e.g. runner_logs is almost cloud agnostic).

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this is independent of the type of cloud, as it is required to schedule reconciliation.

Copy link
Collaborator

@yhaliaw yhaliaw left a comment

Choose a reason for hiding this comment

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

Maybe name the module something like local_lxd, as the module seems to be focused on running runners locally.

# Copyright 2024 Canonical Ltd.
# See LICENSE file for licensing details.

"""Module for managing local LXD cloud."""
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would this module be local LXD specific or LXD in general?
We might need to support remote LXD cloud in the future.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I was thinking of having an LXD module and separating it via local/remote flag since many of the code within them will be shared. What do you think?

Copy link
Collaborator

Choose a reason for hiding this comment

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

The local runners needs some logic specific to it, e.g., firewall setup, running a local repo-policy-compliance, etc.

Would it be good for to have the following modules with a eventual RunnerManager interface implementation:

  • openstack cloud
  • lxd cloud
  • local lxd

and modules for the APIs (might be a single file):

  • openstack API
  • lxd API
  • github API

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sounds good 👍

@yanksyoon yanksyoon closed this Jun 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants