Skip to content

[WIP] feat : Add logic in manager to monitor Pending PipelineRuns and run them#93

Draft
rohanKanojia wants to merge 6 commits into
devtools-qe-incubator:mainfrom
rohankanojia-forks:pr/list-pipelineruns-and-update-status
Draft

[WIP] feat : Add logic in manager to monitor Pending PipelineRuns and run them#93
rohanKanojia wants to merge 6 commits into
devtools-qe-incubator:mainfrom
rohankanojia-forks:pr/list-pipelineruns-and-update-status

Conversation

@rohanKanojia

@rohanKanojia rohanKanojia commented Apr 2, 2025

Copy link
Copy Markdown

Description

⚠️ This is still a work in progress. It's only created to gather early feedback and decide on some unclear implementation aspects. Once I get more clarity I'll refine it further and add unit tests.

  • Add a method in manager.go to list all PipelineRuns in Pending status every 5 seconds
    • As of now, all pending PipelineRuns will be started immediately.

This PR is based on my discussion with @albfan :

  • list pipelineruns in PENDING
  • evaluate which can be started (by now all of them, later depending on non busy machines)
  • do something when each ends (by now nothing at end, mark machines as ready again)
  • Add some control on machines to confirm they are not busy (just if someone start a job manually without eventmanager)
  • inspect too annotations or labels on running pipelineruns, and attach a listener for them (to confirm when they finish)
  • on pipelinerun finish, free related machines (in internal status from eventmanager)
  • design the data structure for exising machines and how to decide what machines need each test (e.g.)
    • mac needs mac-brno1 to build and mac-brno4 to test
    • windows needs windows-blr to build and windows-1-brno to test
    • linux uses linux-1-brno to build and test (it builds on a container)

Signed-off-by: Rohan Kumar rohaan@redhat.com

- Add method in manager.go to list all PipelineRuns in Pending status every 5 seconds
  - as of now all pending PipelineRuns would be started immediately.
Signed-off-by: Rohan Kumar <rohaan@redhat.com>
Signed-off-by: Rohan Kumar <rohaan@redhat.com>
Signed-off-by: Rohan Kumar <rohaan@redhat.com>
@rohanKanojia
rohanKanojia force-pushed the pr/list-pipelineruns-and-update-status branch from 8cd20fd to b31b448 Compare April 7, 2025 14:02
Comment thread pkg/manager/manager.go Outdated
DefaultPipelineRunSyncInterval = 5 * time.Second
)

type BareMetalMachineConfig struct {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I guess it is too early but this logic (hard part of the logic) should be on its own package...the manager should kept as much clean as possible

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I apologize for the messy state of this PR. It's still in a poc-ish state.

Let me at least move this portion to a separate package.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

yeah yean no need to apologies, I saw the WIP you can keep it as much messy as you want till it works and you start to cleanup it up.

Just went through it and added the comment. Feel free to manage it whenever you want not right now

@rohanKanojia rohanKanojia Apr 8, 2025

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I've reorganized the code into a separate package named autoscheduling (not 100% sure about the naming)

pkg/manager/autoscheduling
├── baremetal.go                      # Handles connection and status retrieval for QE lab instances  
└── pipelinerun_manager.go            # Manages status updates for Pending/Completed PipelineRuns  

Let me know your thoughts on the package structure and naming.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Yeah that seems perfect, may I even reduce the name to scheduling or scheduler and then inside the type of resource scheduled in this case pipelines but totally up to you.

Signed-off-by: Rohan Kumar <rohaan@redhat.com>
…nstead of environment variables

Signed-off-by: Rohan Kumar <rohaan@redhat.com>
…stion

Signed-off-by: Rohan Kumar <rohaan@redhat.com>
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