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

Enable ARM32 E2E Test After Mitigating Docker Network Issue #5561

Merged
merged 9 commits into from
Sep 29, 2021

Conversation

nimanch
Copy link
Contributor

@nimanch nimanch commented Sep 22, 2021

All Failures in ARM32 were occuring due to failure of a Module to be installed due to the container being previously attached to the aziot docker network. This is a known bug in docker docker/cli#1891.

The problem is mitigated by restarting docker service on uninstall of edge daemon during teardown sequence.

@nimanch nimanch changed the title Try Enabling Arm32 Enable ARM32 E2E Test After Mitigating Docker Network Issue Sep 23, 2021
@nimanch nimanch marked this pull request as ready for review September 23, 2021 05:51
@@ -108,11 +108,13 @@ public string[] GetInstallCommandsFromLocal(string path)
{
SupportedPackageExtension.Deb => new[]
{
"apt-get purge --yes aziot-edge aziot-identity-service libiothsm-std iotedge"
"apt-get purge --yes aziot-edge aziot-identity-service libiothsm-std iotedge",
Copy link
Contributor

Choose a reason for hiding this comment

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

Since this is a release/1.1 branch. The removal of aziot-* are not necessary -- it doesn't hurt to have them as a precaution. If you want to keep the aziot removal here, please use aziot-* instead of specifying the name 2/4 of the existing aziot components.

Copy link
Contributor Author

@nimanch nimanch Sep 27, 2021

Choose a reason for hiding this comment

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

I dont think I have made any changes to that line of code , the only change was to add the restart docker command. Would you want me to make this change in this PR or create a separate PBI to update this?

Copy link
Contributor

Choose a reason for hiding this comment

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

Would you want me to make this change in this PR or create a separate PBI to update this?

Yes please.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@@ -42,6 +43,17 @@ public async Task BeforeAllAsync()

// Install IoT Edge, and do some basic configuration
await this.daemon.UninstallAsync(token);

// Delete directories used by previous installs.
List<string> directories = new List<string>() { "/run/iotedge", "/var/lib/iotedge", "/var/run/iotedge" };
Copy link
Contributor

Choose a reason for hiding this comment

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

These paths are only true for release/1.1 branch. If you want to be inclusive to 1.2, the var/lib/aziot/*, /var/run/aziot/*, /etc/aziot are needed to be removed as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe that is handled is 1.2 and master branches and the intention is to keep this change isolated to only 1.1 branch and not merge it back to master

@kodiakhq kodiakhq bot merged commit 995083d into Azure:release/1.1 Sep 29, 2021
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.

None yet

2 participants