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

WIP: Refactor master cfg #673

Closed
wants to merge 16 commits into from
Closed

WIP: Refactor master cfg #673

wants to merge 16 commits into from

Conversation

cvicentiu
Copy link
Member

This pull request is built on top of the code for:

#637
#638
#643

The purpose of this commit is to simplify the master.cfg definitions.

@cvicentiu cvicentiu force-pushed the refactor_master_cfg branch 7 times, most recently from b5ddc95 to da131b1 Compare January 8, 2025 14:59
@RazvanLiviuVarzaru
Copy link
Collaborator

RazvanLiviuVarzaru commented Jan 9, 2025

@cvicentiu regarding the failure on bbm-deploy workflow.
This is because the first check in validate_master_cfg is doing a checkconfig on the raw (root level) master.cfg / buildbot.tac which then becomes the base of all autogen/* configurations.

I see two potential solutions:

  1. Remove that block from validate_master script. I can't see why it's useful, any modification to the raw tac / cfg files can be tested through autogen

Proposed removal of:

$RUNC run -i -v "$(pwd):/srv/buildbot/master" \
  -w /srv/buildbot/master \
  $IMAGE \
  buildbot checkconfig master.cfg
echo -e "done\n"

One interesting note, this is why master-config.yml exists in the root level, to make that check pass.
We can remove it as well if solution 1 sounds reasonable.

  1. Alter tac / cfg in define_masters.py - not so elegant

@cvicentiu cvicentiu force-pushed the refactor_master_cfg branch 4 times, most recently from cdd1653 to 7e4e702 Compare January 12, 2025 06:20
cvicentiu and others added 5 commits January 12, 2025 08:35
* Remove star imports
* master-private.cfg always loaded from base directory for all masters.
buildbot.tac as the entrypoint for a buildbot master should use relative
paths based on the deployment directory structure. Furthermore, the
basedir should be set to <srcdir> so that all imports will work relative
to <srcdir>, not the location of buildbot.tac or master.cfg.
Simplify and ensure it returns a 3-value tuple.
This will remove code duplication across master.cfg files.
@cvicentiu cvicentiu force-pushed the refactor_master_cfg branch 3 times, most recently from b14af51 to 147cadd Compare January 12, 2025 07:01
@cvicentiu cvicentiu force-pushed the refactor_master_cfg branch from 147cadd to 171c728 Compare January 12, 2025 07:14
@cvicentiu cvicentiu force-pushed the refactor_master_cfg branch from 8858fbd to 8ad133a Compare January 12, 2025 07:30
* Also fixup unescaped backslashes in strings
@cvicentiu cvicentiu force-pushed the refactor_master_cfg branch from 73f207f to 67d005c Compare January 12, 2025 11:49
@cvicentiu cvicentiu force-pushed the refactor_master_cfg branch from 67d005c to 9e14038 Compare January 12, 2025 11:52
* Also simplify branch matching functions
@cvicentiu cvicentiu force-pushed the refactor_master_cfg branch from 8df173d to c70e9a1 Compare January 12, 2025 12:23
cvicentiu and others added 3 commits January 12, 2025 14:25
* os_info.yaml now provides tags / os
* os_info.yaml specifies directly the image tag. This removes the need
  to do transformations within buildbot configuration code.
* define_masters no longer generates a custom buildbot.tac by replacing
  the log file for the master with a custom name. buildbot.tac now
  uses a log file based on the master directory.
* master-config.yaml now groups builders by architecture.
@cvicentiu cvicentiu force-pushed the refactor_master_cfg branch from 00b902c to bb80148 Compare January 12, 2025 18:10
@cvicentiu
Copy link
Member Author

Pr to be recreated from FORK so CI does not run twice.

@cvicentiu cvicentiu closed this Jan 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants