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

remote/coordinator: Fix typo to allow for reservations matching multiple tags #1491

Merged

Conversation

enkiusz
Copy link
Contributor

@enkiusz enkiusz commented Sep 3, 2024

Description

The code which is creating Reservation objects in the coordinator contains what looks to be a typo. This typo creates a bug where reservations that match more than one tag are not correctly handled.

The places needed to reproduce this problem can be created in the following way:

labgrid-client -p place1 create
labgrid-client -p place1 set-tags env=prod
labgrid-client -p place1 set-tags board_model=foo

labgrid-client -p place2 create
labgrid-client -p place2 set-tags env=ci
labgrid-client -p place2 set-tags board_model=foo

Before fix:

$ labgrid-client reserve --wait env=ci board_model=foo
Reservation '45JJKBHK1X':
  owner: localhost/m.grela
  token: 45JJKBHK1X
  state: allocated
  filters:
    main: board_model=foo
  allocations:
    main: place1
  created: 2024-09-03 12:46:15.928365
  timeout: 2024-09-03 12:47:15.928397
Waiting for allocation...

place1 is incorrectly allocated where place2 should have been selected. Also the reservation record is clearly missing the env=ci tag.

After fix:

$ labgrid-client reserve --wait env=ci board_model=foo
Reservation 'L9WKOT71I7':
  owner: localhost/m.grela
  token: L9WKOT71I7
  state: allocated
  filters:
    main: env=ci board_model=foo
  allocations:
    main: place2
  created: 2024-09-03 12:57:31.191642
  timeout: 2024-09-03 12:58:31.191690
Waiting for allocation...

Checklist

  • PR has been tested

…ple tags

Signed-off-by: Maciej Grela <m.grela@samsung.com>
Copy link
Member

@Emantor Emantor left a comment

Choose a reason for hiding this comment

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

Yes, definitely a bug, the old create_reservation code looks like this:

    async def create_reservation(self, spec, prio=0.0, details=None):
        filter_ = {}
        for pair in spec.split():
            try:
                k, v = pair.split("=")
            except ValueError:
                return None
            if not TAG_KEY.match(k):
                return None
            if not TAG_VAL.match(v):
                return None
            filter_[k] = v

@Emantor Emantor merged commit f1b2bc3 into labgrid-project:master Sep 3, 2024
9 checks passed
@Emantor
Copy link
Member

Emantor commented Sep 3, 2024

Thanks!

@enkiusz enkiusz deleted the devel/fix-reserve-multiple-tags branch September 3, 2024 15:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants