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

CI - Address Failures #132

Closed
timothystewart6 opened this issue Oct 28, 2022 · 7 comments
Closed

CI - Address Failures #132

timothystewart6 opened this issue Oct 28, 2022 · 7 comments
Labels
bug Something isn't working enhancement New feature or request

Comments

@timothystewart6
Copy link
Contributor

Expected Behavior

CI should not fail as often

Current Behavior

CI seems to fail quiet a bit and I think this is just a side effect of how slow the VMs are in GitHub.

I see a few possibilities here:

  • More retries within tests
    • While this might some some failures, it won't solve all
  • Retries for the whole pipeline
    • This might require brining in a retry action, so we can retry the whole filed job.
  • Self-hosted runners
    • This would allow us to have higher performance nodes than what gitlab offers. I have some free compute and could host some if it comes to this.
@timothystewart6 timothystewart6 added bug Something isn't working enhancement New feature or request labels Oct 28, 2022
@sleiner
Copy link
Contributor

sleiner commented Oct 29, 2022

I think this is just a side effect of how slow the VMs are in GitHub.

@timothystewart6 I assume you are talking about the "Test" workflow?

I did just take a look and found that out of the last 25 runs of the workflow, 12 runs failed:

So, let's look at the problems:

  • conflicts in requirements.txt: These are bound to re-occur in dependabot's merge requests as with a "flat" requirements.txt file, we do not differentiate between direct dependencies (stuff that we directly use) and indirect dependencies (stuff that we do not directly use but our dependencies or other indirect dependencies) use. So if any package in our dependency tree has an upper version limit on any of its dependencies and dependabot updates it, we will get these failures. The solution to this is separating our direct dependencies from the "whole, flat" list of dependencies, e.g. using pip-compile (which is also supported by dependabot).
  • a stricter molecule.yml schema in molecule 4.0.2: In the default and ipv6 scenario's molecule.yml, I have been using YAML anchors so that common properties of the nodes (like OS, hardware ressources) do not have to be repeated. The "anchored" nodes live in a key that ansible-lint does not read out. Since molecule 4.0.2 has changed the schema validation implementation, these additional nodes are now no longer accepted. We can either:
    • Get rid of the anchors and repeat that information.
    • Open up a discussion in the molecule project and talk about if and how our use case would be supported.
  • flaky playbook execution: These failures could potentially be caused by poor performance in the GitLab runners.

@timothystewart6
Copy link
Contributor Author

@sleiner Thank you for the analysis! I am going to spin up some self-hosted runners to see if that eases the failures due to performance.

RE: requirement.txt good to know! I will see if I can straighten this out. I am a fan of freezing the requirements however it might be causing too much churn having depdendabot looking at our child dependencies.

RE: molecule.yml schema, good to know. That's too bad we can't use anchors but I am ok with repeating since it's something that shouldn't change too often.

@sleiner
Copy link
Contributor

sleiner commented Oct 30, 2022

RE: requirement.txt good to know! I will see if I can straighten this out. I am a fan of freezing the requirements however it might be causing too much churn having depdendabot looking at our child dependencies.

If we were not freezing dependencies right now, CI would be broken because of the schema changes in molecule 4.0.2, so I agree that this is sensible for this project. Just to make sure that we are on the same page: With pip-compile, you would still freeze the dependencies, but separate the frozen environment (requirements.txt) from your "known requirements" (requirements.in: only direct dependencies, usually only lower bounds on version numbers, except when you're sure a newer version would break something). Through that separation, you get multiple advantages, like...

  • dependabot can keep the whole frozen environment up to date (currently, if any package introduces a new dependency, it would not be added to requirements.txt - likewise if we do not need some indirect dependency anymore, it would currently not remove it from the list but keep on updating it).
  • dependabot can avoid conflicts in requirements.txt(as seen in the original post): requirements.in contains the actual constraints. When freezing requirements (thus creating requirements.txt, everything else can be changed as needed (usually the latest available and compatible version),
  • Some users might not want to install all of the frozen dependencies into a separate venv, but use the project as part of another venv. For that case, they need to ensure that the direct dependencies are met, but additional constraints on all indirect dependencies will just cause conflicts for them.

@sleiner
Copy link
Contributor

sleiner commented Oct 30, 2022

Also, if this was a "real Python project", one should probably use pyproject.toml instead of requirements.txt as well as a dependency manager that comes closer to the state of the art (in my experience, pdm is currently in the lead, other contestants are poetry and pipenv) - but for the scope of this project (we just need to install some Python tools), the really basic requirements.txt approach with pip-compile seems well-suited to me.

@timothystewart6
Copy link
Contributor Author

I think these can be addressed in separate PRs. Thank you again for your insight!

@timothystewart6
Copy link
Contributor Author

I decided not to pursue self-hosted runners, at least in Kubernetes and for now. #136

@timothystewart6
Copy link
Contributor Author

fixed by #389

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants