-
Notifications
You must be signed in to change notification settings - Fork 2
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
feat: garm runners will be reflected into kubernetes as runner resource #23
Conversation
986d63d
to
7c92935
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks very nice - just few nits
🙏 please squash your commits so we will get nice autogenerated release notes. Otherwise each commit will appear in the release notes as one topic in the feature list
2b4e97e
to
6a30799
Compare
4b66b5b
to
844bec6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please update the existing ADR to make the current implementation clear
After communication with @bavarianbidi, I will move the auth refactoring to a separate PR |
f2df4ef
to
013d6e3
Compare
557993c
to
be1e759
Compare
|
b42cf38
to
310ec43
Compare
310ec43
to
b601b8d
Compare
|
||
func (r *RunnerReconciler) PollRunnerInstances(ctx context.Context, eventChan chan event.GenericEvent) { | ||
log := log.FromContext(ctx) | ||
ticker := time.NewTicker(5 * time.Second) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we have this configurable? maybe it's a nice feature - wdyt?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so one could supply a flag in which interval runners are getting polled?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe I do this in a separate PR, I want to get this feature finally shipped 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as discussed, i'm fine with adding an additional flag in a separate PR
Co-authored-by: Mario Constanti <[email protected]>
Co-authored-by: Mario Constanti <[email protected]>
Co-authored-by: Mario Constanti <[email protected]>
Co-authored-by: Mario Constanti <[email protected]>
c4fcbbc
to
3659c47
Compare
Signed-off-by: Mario Constanti <[email protected]>
with
one can now list and with
one can delete a runner from within a cluster.
Also, if accidently a runner get deleted via cli from within the garm-server itself, der controller cleans up not matching runner CRs in k8s
Todo: