sbidoul / runboat

A simple runbot lookalike on kubernetes. Main goal is replacing the OCA runbot.
MIT License
108 stars 50 forks source link

Support repos with conflicting addons #36

Open rousseldenis opened 2 years ago

rousseldenis commented 2 years ago

https://runboat.odoo-community.org/api/v1/builds/b552cf370-23e2-4ce6-b081-f2a76e741cc9/init-log

All runboat builds are failing on 14.0 branch in sale-workflow.

@sbidoul @lmignon

rousseldenis commented 2 years ago

image

sbidoul commented 2 years ago

This is due to modules having the excludes key in the manifest.

runbot did handle that by parsing the travis configuration (via travis2docker) and retaining only the configuration for the first job, therefore takiing into account the INCLUDE/EXCLUDE variables.

Parsing the CI config in runboat is definitely not an option.

I currently think about two approaches to handle this:

  1. trying to automatically determine a subset of compatible modules and installing them in the database, leaving it up to the user to uninstall some and install others if they want to test another subset
  2. generating multiple builds for single commits

Approach 1 sounds hard to implement (well not that hard, but requires some graph analysis wizardry that should go into manifestoo)

Approach 2 first requires supporting multiple builds for a single commit. There is an embryo of that, since the build settings is a list. Then we need to communicate to runboat the addons groups to put in each build. Which hints at some kind of .runboat.yaml at the repo root. Which opens the door to... versatility but also complexity.

There might be other approaches. Not sure what's best.

rousseldenis commented 2 years ago

This is due to modules having the excludes key in the manifest.

runbot did handle that by parsing the travis configuration (via travis2docker) and retaining only the configuration for the first job, therefore takiing into account the INCLUDE/EXCLUDE variables.

Parsing the CI config in runboat is definitely not an option.

I currently think about two approaches to handle this:

  1. trying to automatically determine a subset of compatible modules and installing them in the database, leaving it up to the user to uninstall some and install others if they want to test another subset
  2. generating multiple builds for single commits

Approach 1 sounds hard to implement (well not that hard, but requires some graph analysis wizardry that should go into manifestoo)

Approach 2 first requires supporting multiple builds for a single commit. There is an embryo of that, since the build settings is a list. Then we need to communicate to runboat the addons groups to put in each build. Which hints at some kind of .runboat.yaml at the repo root. Which opens the door to... versatility but also complexity.

There might be other approaches. Not sure what's best.

What was the process in runbot ?

Shouldn't we in that case see case by case if those kind of modules should be in separate repos (but there can be limitations)?

sbidoul commented 2 years ago

What was the process in runbot ?

Have you read what I wrote ? runbot did handle that by...

Shouldn't we in that case see case by case if those kind of modules should be in separate repos (but there can be limitations)?

That is another option, but incompatible modules in the same repo is moderately frequent in OCA, even if they are not explicitly declared as such in their manifests.

rousseldenis commented 2 years ago

Have you read what I wrote ? runbot did handle that by...

Missed that part :face_in_clouds:

petrus-v commented 2 years ago

Haven't follow that much CI OCA improvement last few years, @sbidoul does the goal of runboat to replace runbot and travis ? I've the feeling that things are tested twice !

A bad option could be to ignore some modules (at least in the mean time)... but this will open the doors of bike-shading !

Matrix build defined in config file seems a nice improvement that could be used for other purpose ! Wondering if there are an easy things to do in the mean time to leave Green PR in the mean time in such case ?

rousseldenis commented 2 years ago

@sbidoul does the goal of runboat to replace runbot and travis ?

The goal was to replace only runbot as it was quite difficult to maintain and was based on non-standard flows.

Travis will remain obviously for tests

sbidoul commented 2 years ago

does the goal of runboat to replace runbot and travis ? I've the feeling that things are tested twice !

Runboat is not for running tests. Only installation for manual testing.

Travis is being replaced by GitHub actions for running tests and linters.

petrus-v commented 2 years ago

Oh sorry haven't takes time to deep into details this happens while installing modules :thinking:

And my proposal is some how what runbot is doing today with explicit configuration which you would like to avoid as far I understand !

Thanks for details and all massive works, I've to investigate more to tell something clever on the topic sorry for the nose !

sbidoul commented 2 years ago

@petrus-v no worries! I hope I did not come across as dismissive, it certainly was not my intent.

sbidoul commented 2 years ago

If we decide to implement a .runboat.yaml with an addons matrix, the question of naming the matrix elements will come up. Currently in the oca-addons-repo-template, the copier question about rebel module groups does not ask to name the groups.

Also the matrix would need to be combined with build settings names to create build names.

The names will need to be short enough to reasonably fit in the build cards in the UI.

petrus-v commented 2 years ago

@petrus-v no worries! I hope I did not come across as dismissive, it certainly was not my intent.

I don't feel you were dismissive :). You are very patient and I'm amazed with all you're doing.

rousseldenis commented 2 years ago

If we decide to implement a .runboat.yaml with an addons matrix, the question of naming the matrix elements will come up. Currently in the oca-addons-repo-template, the copier question about rebel module groups does not ask to name the groups.

Also the matrix would need to be combined with build settings names to create build names.

The names will need to be short enough to reasonably fit in the build cards in the UI.

In order to replace definetly runbot by runboat, those questions need to be explored as the current implementation is failing when conflict occurs (even if splitting modules in specific repositories could be a solution, but not universal and time consuming).

sbidoul commented 2 years ago

@rousseldenis or conflict modules can be put in different repos. Besides sale-workflows, are there other places where runboat fails due to this ?

rousseldenis commented 2 years ago

@rousseldenis or conflict modules can be put in different repos. Besides sale-workflows, are there other places where runboat fails du to this ?

I'm not aware of but don't have sight everywhere :sweat_smile:

sbidoul commented 2 years ago

Ok, let us see. As as long as it affects only one repo, my motivation to do this complex change is fairly low.

petrus-v commented 2 years ago

I believe that there are some repos like account-analytic where there are exclude rules in travis (not declared as rebel module in copier) for unit-test there is no issue while installing both modules on the instance but I expect using runboat odoo will failled at some point having modules not compatible.

Not sure who is using runboat and how it is a matter, I guess few people will lose time trying to understand some modules are not compatible ?

my 2cts !

SirTakobi commented 2 years ago

~Another case of incompatible modules: https://github.com/OCA/account-invoicing/pull/1143~ No more conflicting

bealdav commented 1 year ago

OK thanks for this analyze.

What is the situation now ?

There is no duplicate information; nice ! @sbidoul

Could runboat use rebel_module_groups config to install the whole repo except rebel ones without any other setting file ?

If yes, first Stéphane's approach is possible I suppose.

Did I forgot something ?

cc @sbidoul @rousseldenis

sbidoul commented 1 year ago

I want to avoid coupling runboat with unrelated configuration files such as .copier-answers.yml or .github workflows.

A .runboat.yml could provide environment variables that are passed to the containers. For OCA, an EXCLUDE variable could be sufficient as the oca-ci image would then not install these addons in the database, but still make them available for manual installation (IIRC).

The OCA copier template could then populate that with the list of all rebel modules.

I'd welcome a PR exploring that approach.

bealdav commented 1 year ago

Thanks @sbidoul. I don't know if you can define the expected result. But if so, I could check if I can make it works!?

sbidoul commented 1 year ago

One thing that maybe easy to implement and generic at the same time is to let runboat-initialize.sh and runboat-start.sh source $ADDONS_DIR/.env.runboat if it exists.

And then update oca/oca-addons-repo-template to generate .env.runboat with EXCLUDE=rebel modules if needed.

yajo commented 1 year ago

I've hit this bug today in https://github.com/OCA/project/pull/1101.

Keeping in mind the proposal at #94, we could tell runboat to always generate "base" and "all" databases, but we could tell the repo template that, if there are rebel modules, somehow it should tell runboat to skip the "all" database.

bealdav commented 11 months ago

Finally it seems .env.runboat is a convenient way to solve the problem but I'm not sure how to implement it as described by @sbidoul. As said by @yajo, if there are rebel modules we could skip all database.

Moreover it could be appreciate to have en empty db to only install the use case that you want without a bloated UI.

I don't dare to propose to only have an empty db because it's an radical change but it could have advantages :

Then to summarize

1/ add additional runboat install with base module and some code to add to manage it 2/ replace all by base

Any of these choices are convenient for me, but I think we should move because I'm blocked here https://github.com/OCA/stock-logistics-workflow/pull/1271

Thanks a lot

sbidoul commented 11 months ago

I won't have much time to work on this in the coming weeks, but if anyone wants to contribute I'll be happy to help at the OCA Days sprint in Liège. At this point I have a preference for https://github.com/sbidoul/runboat/issues/94 and its associated draft PR rather than .env.runboat (although new requirements may still change my mind).

bealdav commented 11 months ago

Ok, nice I haven't seen https://github.com/sbidoul/runboat/pull/95.

Thanks for that.

As I understand your code if we merge it in this state, we always have db initialized base and all, and it should be sufficient to make PRs with rebel modules green and mergeable . But i understand you want to go further. Unfortunately I can't be in Liège this year.

Thanks a lot

sbidoul commented 11 months ago

and it should be sufficient to make PRs with rebel modules green and mergeable

Note that even today, a red runboat is not blocking merge. It just means the PR is not testable on runboat.

Unfortunately I can't be in Liège this year.

Oh, too bad.