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

Write tests for a dev container setup #18

Closed
3 tasks done
StevenMaude opened this issue Feb 28, 2024 · 9 comments · Fixed by opensafely-core/research-template-docker#13
Closed
3 tasks done

Write tests for a dev container setup #18

StevenMaude opened this issue Feb 28, 2024 · 9 comments · Fixed by opensafely-core/research-template-docker#13
Assignees
Labels
development Tasks involving software development

Comments

@StevenMaude
Copy link

StevenMaude commented Feb 28, 2024

As far as I'm aware, we can't run CI tests in Codespaces somehow. However, it is possible to use dev containers in CI, so validating that a demonstration project at least runs with our configuration might be possible. (EDIT: Codespaces can be spun up using the GH CLI - see comment below - so it might be possible to use this in CI)

It would be good to be constantly assured that whatever dev container setup we go with continues to work, or we catch any problems as they arise.

This should be added into the new repo (see #46 ).

Tasks

  • Run opensafely run run_all -f
  • Schedule this to run regularly if we get it working.
  • Add an alert into Slack if a regular run fails.

Assumptions

That a local dev container setup does work the same as in Codespaces (see #19).

Acceptance criteria

  • Automated tests running in a GitHub workflow using dev containers (not Codespaces).
  • We're testing dev container setup/configuration in the research-template repo works. The tests need to run the opensafely cli tool and check it exists successfully. These tests should be run on a daily schedule.
  • We're testing that the docker image works as expected, with the correct python and R packages present. These tests should be run when we make changes to the research-template-docker repo.
@StevenMaude StevenMaude added this to the Post-discovery ideas milestone Feb 28, 2024
@StevenMaude StevenMaude changed the title Try and write tests for a dev container setup Write tests for a dev container setup Feb 28, 2024
@StevenMaude StevenMaude added the development Tasks involving software development label Feb 28, 2024
@StevenMaude StevenMaude changed the title Write tests for a dev container setup Write tests for a dev container/Codespaces setup Apr 23, 2024
@StevenMaude
Copy link
Author

StevenMaude commented Apr 23, 2024

It's also possible to send commands to a codespace via SSH using the GitHub CLI, which is another possible route to testing, and specifically could test the codespace.

@Jongmassey
Copy link

Jongmassey commented May 8, 2024

The other bespoke aspects of the container setup that might be worth testing are:

  • are all the R packages installed? check nrow(installed.packages())
  • are all the Python packages installed?
  • RStudio server starts correctly and is connectable on port 8787

@Jongmassey
Copy link

a slight snag with this is that the opensafely CLI is installed in a script which is in the research template not the dockerfile

@Jongmassey
Copy link

We can get the expected R package count like this

$ curl -s https://raw.githubusercontent.com/opensafely-core/r-docker/main/renv.lock | jq  '.Packages | length'
424

@Jongmassey
Copy link

here's a better way of comparing

opensafely_r_packages=$(curl -s https://raw.githubusercontent.com/opensafely-core/r-docker/main/renv.lock | jq  '.Packages | keys')
r_installed=$(Rscript -e 'installed_packages <- as.data.frame(installed.packages()[,c(1,3:4)])' -e 'package_names<-unique(installed_packages[installed_packages$Priority!="base"|is.na(installed_packages$Priority),,drop=FALSE][["Package
"]])' -e 'cat(package_names[! package_names %in% c("docopt","spatial","littler")],sep="\n")')

docopt and littler are CLI integration packages that are not included in the renv used in the opensafely R image but are required to make things work in the codespace. spatial I'm not so sure of. I'll test to see whether its exclusion from this comparison is legitimate

@Jongmassey
Copy link

> library('spatial')
Error in library("spatial") : there is no package calledspatial

within opensafely exec r R - not sure where this package has come from. There's a possibility that a user might see this in the locally available packages and then not be able to use it within the opensafely tooling.

@Jongmassey
Copy link

OK the issues with unexpected packages in R was actually a bug in the Dockerfile, for which there's a PR to fix it

@StevenMaude
Copy link
Author

StevenMaude commented May 16, 2024

Note

The discussion in this comment discusses what would be needed for a Codespaces-based test, where a codespace is launched with the image to be tested, and then all newly created resources cleaned up afterwards.

After discussion with Lucy, we decided that this probably wasn't required for now. GitHub are using the dev container setup to create the codespace. It might give us some extra assurance that everything runs in a codespace, but the differences between the two are assumed small enough that the extra work here is not worthwhile at this time.

Trying out testing in Codespaces

I think the dev container test setup is just about done, so I've been quickly trying out running in Codespaces a bit more.

It's possible to run dev containers entirely self-contained within a GitHub Actions runner, making this simpler to test. With dev containers, it's fairly simple to set things up for testing; for example, ensuring the test dev container runs the Docker image that we're intending to test.

Codespaces is primarily a feature with real user interaction in mind, making all this more difficult.

Here are some of the issues I've thought of, or encountered.

Authentication

  • The repository in which we want to create a codespace — opensafely/research-template is separate from opensafely-core/research-template-docker; the default GITHUB_TOKEN in GitHub Actions won't allow access to the other repository?
  • Maybe it's possible to create a, or use an existing, bot account and use a token to create and access a codespace?
  • Edit: potentially one route to working around this would be to create a branch in the research-template-docker repository with the .devcontainer directory copied in from the research-template; which would avoid working with two repositories.

SSH setup

The only way to run commands in a codespace is via SSH. This itself has a few problems:

  • I haven't checked (because I've not got anything working yet) but it's possible that the Rocker R image doesn't have an SSH server.
  • If there's no SSH server available in the R image, then we need to either:
    • add that to the image
    • add SSH as one of the devcontainer.json features
  • Adding SSH by default may not be a good idea, because it's another means to access the codespace which could contain released-but-unpublished data. Although the codespace should be secured by SSH keys.
  • It's not entirely clear if the GitHub CLI sets up SSH keys magically for you, or you need to add these.
  • If we don't add SSH by default, then we need to add it temporarily somehow. That means creating a specific branch based from the current research-template main and adding this option in.

Docker image

  • If we setup a Codespace from the research-template repository, it will by default pull the v0 image. Unlike with dev containers, this is irrespective of building a v0 image in the Actions workflow and using that image because it's already present. All this presumably happens externally via an API call triggered by the GitHub CLI.
  • How would we get the version of the image into the devcontainer.json? One way — with a lot of potential detritus — might be:
    • build an image and tag it as :vtest-$DATETIME or similar
    • push the image to GitHub
    • create a new branch based on main in the research-template repository test-$DATETIME or similar
    • substitute the image version in devcontainer.json for :vtest-$DATETIME in devcontainer.json in that new test branch
    • run a codespace based on that new test branch
    • delete the test branch when done
    • delete the test Docker image
  • Again, you'd need to authenticate with an account that has write access to the other repository (see above).
  • I don't think there's a way to configure something like an environment variable to specify the devcontainer.json image and override the image version that we use.
  • What if someone changes main in the meantime in a way that's incompatible, meaning the tests pass when you try them, but not when they've made their changes?
  • What if GitHub Actions glitches mid-run and doesn't clean up resources? We could start by trying to delete images and branches older than, say, 2 days or 1 week.

Billing

  • I don't think billing is a major issue. Codespaces gives free accounts 60 hours per month on the lowest specification machine type.
  • Again, it's probably more sensible for a bot account to run such tests, if it's possible to even run them.
  • If we assume that a test takes 5 minutes of Codespaces quota, then with the free 60 hours Codespaces usage for the basic machine, we could potentially run 12 tests in an hour's quota and 60 * 12 = 720 tests per month.
  • We could reduce the usage by running the Codespaces test daily on a schedule, and perhaps pushes to main, instead of every pull request push.
  • It's possible to try and enforce clean up of the codespace used in a test with if: always() and similar in GitHub Actions. It may still be possible for clean up to fail if GitHub Actions glitches mid-run.
  • It's possible to use short idle timeout and retention time, to mitigate against failing to clean up the codespace.

Do we still want a Codespaces-based test?

Once opensafely-core/research-template-docker#13 is merged, we'll have automated testing of the dev container.

The benefit of having a Codespaces-based test is a slightly stronger assurance that the dev container setup works not just generally, but in GitHub's environment where researchers will typically work.

Whether that assurance is worth this extra complexity now is what we should decide. I think it's certainly possible to make this, but is this — slightly messier, and potentially less reliable — test worth adding right now to cover that? Or should we have some evidence of a problem that would be caught by it, but not by the existing dev container test first?

@lucyb lucyb changed the title Write tests for a dev container/Codespaces setup Write tests for a dev container setup May 21, 2024
@StevenMaude
Copy link
Author

Adding a Slack notification is covered by opensafely-core/research-template-docker#21.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
development Tasks involving software development
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants