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

Regular maintenance updates for Linux VMs #1885

Merged
merged 33 commits into from
May 16, 2024

Conversation

craddm
Copy link
Contributor

@craddm craddm commented May 13, 2024

✅ Checklist

  • You have given your pull request a meaningful title (e.g. Enable foobar integration rather than 515 foobar).
  • You are targeting the appropriate branch. If you're not certain which one this is, it should be develop.
  • Your branch is up-to-date with the target branch (it probably was when you started, but it may have changed since then).
  • You have marked this pull request as a draft and added '[WIP]' to the title if needed (if you're not yet ready to merge).

⤴️ Summary

This PR adds regular OS updates for Linux VMs in SREs.

It creates a maintenance configuration in the workspaces resource group for the SRE, and then associates Linux VMs in that group with the configuration.

It is set up to install any critical or security patches daily, at 1am.

Also fixes a bug preventing deployment of multiple workspaces.

🌂 Related issues

Closes #1889

🔬 Tests

Succesfully deployed an SRE

Copy link

github-actions bot commented May 13, 2024

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  data_safe_haven/commands
  sre.py
  data_safe_haven/external/interface
  azure_ipv4_range.py
  data_safe_haven/functions
  strings.py
  data_safe_haven/infrastructure/common
  ip_ranges.py
  transformations.py
  data_safe_haven/infrastructure/components/composite
  virtual_machine.py 47, 287
  data_safe_haven/infrastructure/programs
  declarative_shm.py
  declarative_sre.py 333
  data_safe_haven/infrastructure/programs/shm
  networking.py
  data_safe_haven/infrastructure/programs/sre
  maintenance.py 20-22, 36-70
  networking.py
  workspaces.py 66, 185
  data_safe_haven/types
  enums.py
  tests/functions
  test_strings.py
Project Total  

This report was generated by python-coverage-comment-action

@craddm craddm changed the title [WIP] Regular maintenance updates for Linux VMs Regular maintenance updates for Linux VMs May 14, 2024
@craddm craddm marked this pull request as ready for review May 14, 2024 14:25
@craddm craddm requested a review from a team as a code owner May 14, 2024 14:25
craddm and others added 3 commits May 14, 2024 15:28
@jemrobinson
Copy link
Member

If the only usage of the SREMaintenanceComponent is in workspaces and the SREMaintenanceComponent only contains a single resource (namely the automation.SoftwareUpdateConfigurationByName object) then it might make more sense to move this resource into workspaces.py and drop the separate monitoring.py component.

If there's a good reason to keep them separate (e.g. because we're planning to add other VMs later) then let's discuss that here.

@craddm
Copy link
Contributor Author

craddm commented May 14, 2024

If the only usage of the SREMaintenanceComponent is in workspaces and the SREMaintenanceComponent only contains a single resource (namely the automation.SoftwareUpdateConfigurationByName object) then it might make more sense to move this resource into workspaces.py and drop the separate monitoring.py component.

Do you mean just fold the maintenance component into workspaces.py, losing the separate maintenance.py and dropping monitoring.py completely? automation.SoftwareUpdateConfigurationByName isn't part of the maintenance component, but it is superseded by it.

@jemrobinson
Copy link
Member

Do you mean just fold the maintenance component into workspaces.py, losing the separate maintenance.py and dropping monitoring.py completely? automation.SoftwareUpdateConfigurationByName isn't part of the maintenance component, but it is superseded by it.

Yes.

  • monitoring.py registers the VMs with the previous LogAnalytics/AzureAutomation setup and I think we don't need it
  • maintenance.py should be either:
    • a component that's created in declarative_sre.py with the configuration ID passed to workspaces
    • nothing, with the maintenance.MaintenanceConfiguration moved to workspaces.py instead

the second option here only applies if we're sure that everything that might want to use the MaintenanceConfiguration is in workspaces.py. This is currently true, but there might be reasons why we're planning to add new VMs (not container images) in the future (I can't think of any but maybe you or @JimMadge can?).

@craddm
Copy link
Contributor Author

craddm commented May 15, 2024

Do you mean just fold the maintenance component into workspaces.py, losing the separate maintenance.py and dropping monitoring.py completely? automation.SoftwareUpdateConfigurationByName isn't part of the maintenance component, but it is superseded by it.

Yes.

  • monitoring.py registers the VMs with the previous LogAnalytics/AzureAutomation setup and I think we don't need it

Agreed

  • maintenance.py should be either:

    • a component that's created in declarative_sre.py with the configuration ID passed to workspaces
    • nothing, with the maintenance.MaintenanceConfiguration moved to workspaces.py instead

the second option here only applies if we're sure that everything that might want to use the MaintenanceConfiguration is in workspaces.py. This is currently true, but there might be reasons why we're planning to add new VMs (not container images) in the future (I can't think of any but maybe you or @JimMadge can?).

No, can't think of any either. Leaning towards this option. Easy to put it back to the current style if it makes sense in the future.

craddm and others added 4 commits May 15, 2024 13:26
…chine.py

Co-authored-by: James Robinson <james.em.robinson@gmail.com>
…chine.py

Co-authored-by: James Robinson <james.em.robinson@gmail.com>
…chine.py

Co-authored-by: James Robinson <james.em.robinson@gmail.com>
Copy link
Member

@JimMadge JimMadge left a comment

Choose a reason for hiding this comment

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

LGTM.

I don't have particularly strong opinions about moving the MaintenanceConfiguration to workspaces.py.

On one hand, that would mean less boiler plate.
On the other, it would make one file longer and remove the nice modular feel it has and would be more scalable if we need multiple instances or to register other objects to it.

Copy link
Member

@jemrobinson jemrobinson left a comment

Choose a reason for hiding this comment

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

LGTM

@jemrobinson jemrobinson merged commit a71f3a9 into alan-turing-institute:develop May 16, 2024
11 checks passed
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.

Duplicate resource name when deploying multiple workspaces
3 participants