Skip to content
This repository has been archived by the owner on Sep 8, 2024. It is now read-only.

Feature: enable simultaneous CI jobs and keep logs #2762

Merged
merged 12 commits into from
Dec 29, 2020

Conversation

krisgesling
Copy link
Contributor

Description

  • When multiple jobs run Voight Kampff tests simultaneously they are currently writing the allure data to the same directory on the host. This can create clear conflicts. These are now stored under unique directories $HOME/core/PR-####/(allure|mycroft-logs) that are cleaned up on each run.
  • This PR also retains mycroft-logs and provides them on test failure as is now done for mycroft-skills.
  • Adds a lockable resource with the repo and PR number to ensure that the directory won't be used by another job from the same PR. I don't think this can happen anyway, but it seemed like an easy fail safe.
  • Finally we improved the cleanup of docker containers by explicitly removing containers generated by this job if the build was successful. Based on: Add post processing to Jenkins to clean up Docker containers and images. selene-backend#230

How to test

Replay all the jobs!

As the Docker images now store Allure and log files in unique
directories, multiple CI jobs from the same repository can now run
simultaneously.
This does not include the Major release Docker image as it is
intentionally built for re-use.
@devs-mycroft devs-mycroft added the CLA: Yes Contributor License Agreement exists (see https://github.com/MycroftAI/contributors) label Nov 13, 2020
@devops-mycroft
Copy link

Voight Kampff Integration Test Failed (Results).
Mycroft logs are also available: skills.log, audio.log, voice.log, bus.log, enclosure.log

Move all outputs of a CI job into the same directory
@krisgesling krisgesling added Status: To be reviewed Concept accepted and PR has sufficient information for full review Type: Enhancement - roadmapped Implementation of a feature that is a priority on the roadmap. labels Nov 14, 2020
@devops-mycroft
Copy link

Voight Kampff Integration Test Failed (Results).
Mycroft logs are also available: skills.log, audio.log, voice.log, bus.log, enclosure.log

@forslund
Copy link
Collaborator

The big issue here is sharing of the Mycroft identity. If multiple mycroft instances shares it it may be saved in an invalid state if the identity is updated.

@krisgesling
Copy link
Contributor Author

In this instance I think it was just a directory permissions issue.
As I hadn't created the directories in the script, when binding them with Docker it created them automatically but with incorrect permissions.
So I've added a mkdir -p to explicitly create them and removed the directories that were previously created on the host.

I haven't yet seen the identity issue surface. It is something we should address but doesn't seem to be a blocker just yet.

@devops-mycroft
Copy link

Voight Kampff Integration Test Succeeded (Results)

@forslund
Copy link
Collaborator

I didn't mean that it was the cause of the failure. I just pointed out a potential hurdle here. (it has historically been an issue)

@devops-mycroft
Copy link

Voight Kampff Integration Test Succeeded (Results)

krisgesling and others added 4 commits December 9, 2020 14:13
If Mimic was not found at all the LOG message would raise an exception,
this fixes that issue and cleans up Exception chain
Verify that the fallback object can execute when creating it
@devops-mycroft
Copy link

Voight Kampff Integration Test Succeeded (Results)

@krisgesling krisgesling merged commit 0050bde into dev Dec 29, 2020
@krisgesling krisgesling deleted the feature/improve-concurrent-ci branch December 29, 2020 00:36
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CLA: Yes Contributor License Agreement exists (see https://github.com/MycroftAI/contributors) Status: To be reviewed Concept accepted and PR has sufficient information for full review Type: Enhancement - roadmapped Implementation of a feature that is a priority on the roadmap.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants