Skip to content

feat: add ownedBy and createdAt for OpenModel #438

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

googs1025
Copy link
Member

What this PR does / why we need it

  • add ownedBy and createdAt for OpenModel

Which issue(s) this PR fixes

Fixes #435

Special notes for your reviewer

Does this PR introduce a user-facing change?

add ownedBy and createdAt for OpenModel CR

@InftyAI-Agent InftyAI-Agent added needs-triage Indicates an issue or PR lacks a label and requires one. needs-priority Indicates a PR lacks a label and requires one. do-not-merge/needs-kind Indicates a PR lacks a label and requires one. labels Jun 3, 2025
@googs1025 googs1025 marked this pull request as draft June 3, 2025 08:06
@InftyAI-Agent InftyAI-Agent requested review from kerthcet June 3, 2025 08:06
@googs1025
Copy link
Member Author

will test locally soon

@googs1025 googs1025 force-pushed the feature/OpenModel_field branch 3 times, most recently from cc8eca3 to 55df089 Compare June 3, 2025 09:17
@googs1025
Copy link
Member Author

root@VM-0-3-ubuntu:/home/ubuntu/llmaz/docs/examples/ollama# kubectl get openmodel -oyaml
apiVersion: v1
items:
- apiVersion: llmaz.io/v1alpha1
  kind: OpenModel
  metadata:
    annotations:
      kubectl.kubernetes.io/last-applied-configuration: |
        {"apiVersion":"llmaz.io/v1alpha1","kind":"OpenModel","metadata":{"annotations":{},"name":"qwen2-0--5b"},"spec":{"familyName":"qwen2","source":{"uri":"ollama://qwen2:0.5b"}}}
    creationTimestamp: "2025-06-03T13:30:42Z"
    generation: 2
    labels:
      llmaz.io/model-family-name: qwen2
    name: qwen2-0--5b
    resourceVersion: "3586"
    uid: 9df4445e-fbbb-4083-b3ff-4591dfc1f95e
  spec:
    createdAt: "2025-06-03T13:30:42Z"
    familyName: qwen2
    ownedBy: llmaz
    source:
      uri: ollama://qwen2:0.5b
kind: List
metadata:
  resourceVersion: ""
root@VM-0-3-ubuntu:/home/ubuntu/llmaz/docs/examples/ollama#

@googs1025 googs1025 marked this pull request as ready for review June 3, 2025 13:32
@googs1025
Copy link
Member Author

/kind feature

@InftyAI-Agent InftyAI-Agent added feature Categorizes issue or PR as related to a new feature. and removed do-not-merge/needs-kind Indicates a PR lacks a label and requires one. labels Jun 3, 2025
@@ -73,3 +85,22 @@ func (r *ModelReconciler) SetupWithManager(mgr ctrl.Manager) error {
For(&coreapi.OpenModel{}).
Complete(r)
}

// needsUpdated checks if the model needs to be updated.
func needsUpdated(model *coreapi.OpenModel) (bool, *coreapi.OpenModel) {
Copy link
Member

Choose a reason for hiding this comment

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

Move them to the webhooks which I believe is the right place. And I think we don't need to set the OwnedBy since we have kubebuilder tag.

Copy link
Member Author

Choose a reason for hiding this comment

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

sgtm. I forget that we have OpenModelWebhook... I will change this part

Copy link
Member Author

@googs1025 googs1025 Jun 5, 2025

Choose a reason for hiding this comment

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

Back to this again, if I understand correctly, when we put it in the webhook, it seems that model.CreationTimestamp is still empty when we create it. @kerthcet
🤔

func (w *OpenModelWebhook) Default(ctx context.Context, obj runtime.Object) error {
	model := obj.(*coreapi.OpenModel)
	if model.Labels == nil {
		model.Labels = map[string]string{}
	}
	model.Labels[coreapi.ModelFamilyNameLabelKey] = string(model.Spec.FamilyName)
	if model.Spec.CreatedAt == nil {
		model.Spec.CreatedAt = &model.CreationTimestamp
	}
	return nil
}

Copy link
Member

Choose a reason for hiding this comment

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

Yes, we should set it automatically.

@googs1025 googs1025 force-pushed the feature/OpenModel_field branch from 55df089 to dcbc1e8 Compare June 4, 2025 23:56
@googs1025 googs1025 marked this pull request as draft June 5, 2025 00:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Categorizes issue or PR as related to a new feature. needs-priority Indicates a PR lacks a label and requires one. needs-triage Indicates an issue or PR lacks a label and requires one.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add fields ownedBy and createdAt for OpenModel
3 participants