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

Fix: Remove devcontainer mounts to address error 16 with molecule #3541

Merged
merged 10 commits into from
Feb 8, 2024

Conversation

ankudinov
Copy link
Contributor

Change Summary

This PR addresses a bug with molecule tests failing in AVD dev container due to error 16: device or resource busy when bind mount from workspace subdirectory to Ansible collection path is present.
As this bind mount is causing significant confusion and increases dev container complexity - it's feasible to remove this option until we have a real use case.
Container can be always rebuild after the change if required to reinstall the collection.

Component(s) name

AVD dev container

Proposed changes

  • remove mounts from devcontainer.json
  • update comments in the devcontainer.json
  • consider replacing onCreateCommand with postCreateCommand (additional testing required before moving this PR out of draft stage)

How to test

Start dev container from this branch and test if molecule tests are successful.

This reverts commit 85c62e2.

Keep old version of entry point for the following reasons:
1) to avoid additional testing
2) installing collections from the mount can be useful for future custom cases
This can be always reviewed and changed in future.
@ankudinov ankudinov marked this pull request as ready for review February 6, 2024 10:45
@ankudinov ankudinov requested review from a team as code owners February 6, 2024 10:45
@ankudinov
Copy link
Contributor Author

Switched from onCreateCommand and postCreateCommand to postStartCommand. The first 2 are initiating install script 2 early in the process, before VSCode finishes all the tasks, like copying git credentials and marking git repo as trusted with git config --global --add safe.directory /workspaces/ansible-avd. If the collection install script starts too early it fails with fatal: detected dubious ownership in repository at '/workspaces/ansible-avd'.
The downside of postStartCommand is that it is triggered every time when container starts. To prevent triggering installation every time the script now verifies Ansible binary presence first with command -v.

@ankudinov
Copy link
Contributor Author

I'm keeping the logic required to install collections from a bind mount part of the script to cover some special cases that may come in the future and to avoid re-testing the logic. But open for suggestions and alternative opinions.
The mount option will be marked as unsupported later (once images are release) to avoid IO problems like molecule case in the field.

@ClausHolbechArista ClausHolbechArista requested a review from a team February 8, 2024 14:15
@ClausHolbechArista ClausHolbechArista added the one approval This PR has one approval and is only missing one more. label Feb 8, 2024
fix comment

Co-authored-by: Guillaume Mulocher <[email protected]>
@gmuloc gmuloc merged commit 4dd92cb into aristanetworks:devel Feb 8, 2024
38 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
one approval This PR has one approval and is only missing one more. rn: Fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants