Skip to content

Cleanup post transfer #3

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.

Sign up for GitHub

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

Jump to bottom
Merged
antweiss merged 16 commits into from
Sep 17, 2025
Merged

Cleanup post transfer #3

antweiss merged 16 commits into from
Sep 17, 2025
+915 −672

Conversation

@EItanya
Copy link
Collaborator

@EItanya EItanya commented Sep 9, 2025 •
edited
Loading

  • move agentId to agentRef
  • change khook-controller -> khook
  • fix build file

@EItanya
feat: move agentId to agentRef, change khook-controller -> khook, fix…
c6a8d79 
… build file

Signed-off-by: Eitan Yarmush <eitan.yarmush@solo.io>
@EItanya
parallelize
dc72f67 
Signed-off-by: Eitan Yarmush <eitan.yarmush@solo.io>
@EItanya
fix workflow
1a1b3f5 
Signed-off-by: Eitan Yarmush <eitan.yarmush@solo.io>
@EItanya EItanya changed the title feat: move agentId to agentRef, change khook-controller -> khook, fix… Cleanup post transfer Sep 9, 2025
@EItanya
feat: rename kind cluster to kagent to make cross work easier
ede3aa1 
Signed-off-by: Eitan Yarmush <eitan.yarmush@solo.io>
@EItanya
move helm into helm folder
966c5cf 
Signed-off-by: Eitan Yarmush <eitan.yarmush@solo.io>
@EItanya
add helm-version
1b6fcf7 
Signed-off-by: Eitan Yarmush <eitan.yarmush@solo.io>
@EItanya
add push-helm-chart
e2afd26 
Signed-off-by: Eitan Yarmush <eitan.yarmush@solo.io>
@EItanya
update release file, I think it should work
438871b 
Signed-off-by: Eitan Yarmush <eitan.yarmush@solo.io>
@EItanya
use newer version of release job
1f2e212 
Signed-off-by: Eitan Yarmush <eitan.yarmush@solo.io>
antweiss
antweiss requested changes Sep 10, 2025
View reviewed changes
Copy link
Collaborator

@antweiss antweiss left a comment

Choose a reason for hiding this comment

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

Thank you for this massive improvement! Found a few small issues. LMK what you think.

api/v1alpha2/hook_types.go Outdated
// +kubebuilder:validation:Required
EventType string `json:"eventType"`

// AgentId specifies the Kagent agent to call when this event occurs
Copy link
Collaborator

@antweiss antweiss Sep 10, 2025

Choose a reason for hiding this comment

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

Probably need to update the variable type in the comment to AgentRef too.

docs/installation.md Outdated

```bash
TMP_DIR="$(mktemp -d)" && \
git clone --depth 1 https://github.com/antweiss/khook.git "$TMP_DIR/khook" && \
Copy link
Collaborator

@antweiss antweiss Sep 10, 2025

Choose a reason for hiding this comment

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

Need to update the repo URL to https://github.com/kagent-dev/khook

README.md Outdated
One-liner (no checkout):
```bash
TMP_DIR="$(mktemp -d)" && \
git clone --depth 1 https://github.com/antweiss/khook.git "$TMP_DIR/khook" && \
Copy link
Collaborator

@antweiss antweiss Sep 10, 2025

Choose a reason for hiding this comment

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

Please update the repo URL to https://github.com/kagent-dev/khook

examples/ci-cd-monitoring.yaml
kind: Hook
metadata:
name: ci-cd-monitoring
namespace: ci-cd
Copy link
Collaborator

@antweiss antweiss Sep 10, 2025

Choose a reason for hiding this comment

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

This being an example and hooks being namespaced resources I think it actually makes more sense to stay with ns 'ci-cd' - to show how hooks can be created in any namespace.

examples/development-monitoring.yaml
kind: Hook
metadata:
name: dev-monitoring
namespace: development
Copy link
Collaborator

@antweiss antweiss Sep 10, 2025

Choose a reason for hiding this comment

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

Namespace again. I think examples better show hooks in various namespaces.

examples/multi-namespace-monitoring.yaml
kind: Hook
metadata:
name: staging-monitoring
namespace: staging
Copy link
Collaborator

@antweiss antweiss Sep 10, 2025

Choose a reason for hiding this comment

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

Namespace

examples/ci-cd-monitoring.yaml
eventConfigurations:
# Monitor pod pending issues in CI/CD environments
- eventType: pod-pending
agentId: kagent/ci-cd-engineer
Copy link
Collaborator

@antweiss antweiss Sep 10, 2025

Choose a reason for hiding this comment

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

And maybe add namespace to agentRef so the example explains how agents in other namespaces can be referenced. (I just realized this myself)

@EItanya
fix: update with new namespaced ref API
192c26c 
Signed-off-by: Eitan Yarmush <eitan.yarmush@solo.io>
@EItanya
change links to kagent-dev repo
f6976d7 
Signed-off-by: Eitan Yarmush <eitan.yarmush@solo.io>
@EItanya
remove unneeded deps
f4c255a 
Signed-off-by: Eitan Yarmush <eitan.yarmush@solo.io>
@EItanya
fixed the integration test
1bf2e76 
Signed-off-by: Eitan Yarmush <eitan.yarmush@solo.io>
@EItanya
update tests for new typed interfaces
a49f684 
Signed-off-by: Eitan Yarmush <eitan.yarmush@solo.io>
antweiss
antweiss reviewed Sep 14, 2025
View reviewed changes
antweiss
antweiss reviewed Sep 14, 2025
View reviewed changes
@antweiss
Copy link
Collaborator

antweiss commented Sep 14, 2025

I pushed a few fixes so that the CRD reflects the agentRef change. But now I'm getting:

Failed to call agent  for event pod-restart on resource baler: failed to create session: HTTP 400: Agent ref is invalid, please check the agent ref : failed to get model: record not found	{"type": "Warning", "object": {"kind":"Hook","namespace":"default","name":"basic-pod-monitoring","uid":"ad3d5638-8899-420c-9c85-862e6d9fd054","apiVersion":"kagent.dev/v1alpha2","resourceVersion":"16000"}, "reason": "AgentCallFailure"}

Need to debug what exactly isn't working here.

@EItanya
Copy link
Collaborator Author

EItanya commented Sep 14, 2025

I pushed a few fixes so that the CRD reflects the agentRef change. But now I'm getting:

Failed to call agent  for event pod-restart on resource baler: failed to create session: HTTP 400: Agent ref is invalid, please check the agent ref : failed to get model: record not found	{"type": "Warning", "object": {"kind":"Hook","namespace":"default","name":"basic-pod-monitoring","uid":"ad3d5638-8899-420c-9c85-862e6d9fd054","apiVersion":"kagent.dev/v1alpha2","resourceVersion":"16000"}, "reason": "AgentCallFailure"}

Need to debug what exactly isn't working here.

I’ll fix this tomorrow

@antweiss @EItanya
Update CRD definitions for agentRef
b1bb919 
Signed-off-by: Eitan Yarmush <eitan.yarmush@solo.io>
@EItanya EItanya force-pushed the eitanya/api-changes branch from dd93621 to b1bb919 Compare September 15, 2025 14:16
@EItanya
use correct ref for session
54ad20e 
Signed-off-by: Eitan Yarmush <eitan.yarmush@solo.io>
@antweiss
Copy link
Collaborator

antweiss commented Sep 17, 2025

Working great now! Thanks a lot @EItanya

@antweiss antweiss self-requested a review September 17, 2025 08:56
antweiss
antweiss approved these changes Sep 17, 2025
View reviewed changes
Copy link
Collaborator

@antweiss antweiss left a comment

Choose a reason for hiding this comment

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

LGTM. thanks @EItanya

@antweiss antweiss merged commit 19089d3 into main Sep 17, 2025
3 checks passed
@antweiss antweiss deleted the eitanya/api-changes branch September 17, 2025 08:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@antweiss antweiss antweiss approved these changes

@yuval-k yuval-k Awaiting requested review from yuval-k yuval-k is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@EItanya @antweiss