diff --git a/controller/api/v1alpha1/lease_helpers.go b/controller/api/v1alpha1/lease_helpers.go index b334eac83..63f81ca6d 100644 --- a/controller/api/v1alpha1/lease_helpers.go +++ b/controller/api/v1alpha1/lease_helpers.go @@ -205,6 +205,12 @@ func LeaseFromProtobuf( ClientRef: clientRef, Duration: duration, Selector: *selector, + ExporterRef: func() *corev1.LocalObjectReference { + if req.ExporterName == nil || *req.ExporterName == "" { + return nil + } + return &corev1.LocalObjectReference{Name: *req.ExporterName} + }(), BeginTime: beginTime, EndTime: endTime, }, @@ -233,6 +239,9 @@ func (l *Lease) ToProtobuf() *cpb.Lease { Client: ptr.To(fmt.Sprintf("namespaces/%s/clients/%s", l.Namespace, l.Spec.ClientRef.Name)), Conditions: conditions, } + if l.Spec.ExporterRef != nil { + lease.ExporterName = ptr.To(l.Spec.ExporterRef.Name) + } if l.Spec.Duration != nil { lease.Duration = durationpb.New(l.Spec.Duration.Duration) } diff --git a/controller/api/v1alpha1/lease_helpers_test.go b/controller/api/v1alpha1/lease_helpers_test.go index 221a7dd37..c9ee9072a 100644 --- a/controller/api/v1alpha1/lease_helpers_test.go +++ b/controller/api/v1alpha1/lease_helpers_test.go @@ -313,5 +313,23 @@ var _ = Describe("LeaseFromProtobuf", func() { Expect(lease).NotTo(BeNil()) Expect(lease.Labels).To(BeEmpty()) // nil or empty map is fine }) + + It("should keep requested exporter name in spec exporterRef", func() { + exporterName := "device-1" + pbLease := &cpb.Lease{ + Selector: "", + Duration: durationpb.New(time.Hour), + ExporterName: &exporterName, + } + key := types.NamespacedName{Name: "test-lease", Namespace: "default"} + clientRef := corev1.LocalObjectReference{Name: "test-client"} + + lease, err := LeaseFromProtobuf(pbLease, key, clientRef) + + Expect(err).NotTo(HaveOccurred()) + Expect(lease).NotTo(BeNil()) + Expect(lease.Spec.ExporterRef).NotTo(BeNil()) + Expect(lease.Spec.ExporterRef.Name).To(Equal(exporterName)) + }) }) }) diff --git a/controller/api/v1alpha1/lease_types.go b/controller/api/v1alpha1/lease_types.go index fdecd42a1..dfd8e9a9d 100644 --- a/controller/api/v1alpha1/lease_types.go +++ b/controller/api/v1alpha1/lease_types.go @@ -21,6 +21,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) +// +kubebuilder:validation:XValidation:rule="((has(self.selector.matchLabels) && size(self.selector.matchLabels) > 0) || (has(self.selector.matchExpressions) && size(self.selector.matchExpressions) > 0)) || (has(self.exporterRef) && has(self.exporterRef.name) && size(self.exporterRef.name) > 0)",message="one of selector or exporterRef.name is required" // LeaseSpec defines the desired state of Lease type LeaseSpec struct { // The client that is requesting the lease @@ -30,7 +31,10 @@ type LeaseSpec struct { // in which case it's calculated as EndTime - BeginTime. Duration *metav1.Duration `json:"duration,omitempty"` // The selector for the exporter to be used + // +kubebuilder:default:={} Selector metav1.LabelSelector `json:"selector"` + // Optionally pin this lease to a specific exporter name. + ExporterRef *corev1.LocalObjectReference `json:"exporterRef,omitempty"` // The release flag requests the controller to end the lease now Release bool `json:"release,omitempty"` // Requested start time. If omitted, lease starts when exporter is acquired. diff --git a/controller/api/v1alpha1/zz_generated.deepcopy.go b/controller/api/v1alpha1/zz_generated.deepcopy.go index 8af9619f6..7f2e90e42 100644 --- a/controller/api/v1alpha1/zz_generated.deepcopy.go +++ b/controller/api/v1alpha1/zz_generated.deepcopy.go @@ -495,6 +495,11 @@ func (in *LeaseList) DeepCopyObject() runtime.Object { func (in *LeaseSpec) DeepCopyInto(out *LeaseSpec) { *out = *in out.ClientRef = in.ClientRef + if in.ExporterRef != nil { + in, out := &in.ExporterRef, &out.ExporterRef + *out = new(v1.LocalObjectReference) + **out = **in + } if in.Duration != nil { in, out := &in.Duration, &out.Duration *out = new(metav1.Duration) diff --git a/controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/crds/jumpstarter.dev_leases.yaml b/controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/crds/jumpstarter.dev_leases.yaml index 9aafc8591..2df4db845 100644 --- a/controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/crds/jumpstarter.dev_leases.yaml +++ b/controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/crds/jumpstarter.dev_leases.yaml @@ -81,11 +81,26 @@ spec: Can be updated to extend or shorten active leases. format: date-time type: string + exporterRef: + description: Optionally pin this lease to a specific exporter name. + properties: + name: + default: "" + description: |- + Name of the referent. + This field is effectively required, but due to backwards compatibility is + allowed to be empty. Instances of this type with an empty value here are + almost certainly wrong. + More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + type: string + type: object + x-kubernetes-map-type: atomic release: description: The release flag requests the controller to end the lease now type: boolean selector: + default: {} description: The selector for the exporter to be used properties: matchExpressions: @@ -135,6 +150,12 @@ spec: - clientRef - selector type: object + x-kubernetes-validations: + - message: one of selector or exporterRef.name is required + rule: ((has(self.selector.matchLabels) && size(self.selector.matchLabels) + > 0) || (has(self.selector.matchExpressions) && size(self.selector.matchExpressions) + > 0)) || (has(self.exporterRef) && has(self.exporterRef.name) && size(self.exporterRef.name) + > 0) status: description: LeaseStatus defines the observed state of Lease properties: diff --git a/controller/deploy/operator/config/crd/bases/jumpstarter.dev_leases.yaml b/controller/deploy/operator/config/crd/bases/jumpstarter.dev_leases.yaml index 9aafc8591..2df4db845 100644 --- a/controller/deploy/operator/config/crd/bases/jumpstarter.dev_leases.yaml +++ b/controller/deploy/operator/config/crd/bases/jumpstarter.dev_leases.yaml @@ -81,11 +81,26 @@ spec: Can be updated to extend or shorten active leases. format: date-time type: string + exporterRef: + description: Optionally pin this lease to a specific exporter name. + properties: + name: + default: "" + description: |- + Name of the referent. + This field is effectively required, but due to backwards compatibility is + allowed to be empty. Instances of this type with an empty value here are + almost certainly wrong. + More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + type: string + type: object + x-kubernetes-map-type: atomic release: description: The release flag requests the controller to end the lease now type: boolean selector: + default: {} description: The selector for the exporter to be used properties: matchExpressions: @@ -135,6 +150,12 @@ spec: - clientRef - selector type: object + x-kubernetes-validations: + - message: one of selector or exporterRef.name is required + rule: ((has(self.selector.matchLabels) && size(self.selector.matchLabels) + > 0) || (has(self.selector.matchExpressions) && size(self.selector.matchExpressions) + > 0)) || (has(self.exporterRef) && has(self.exporterRef.name) && size(self.exporterRef.name) + > 0) status: description: LeaseStatus defines the observed state of Lease properties: diff --git a/controller/internal/controller/lease_controller.go b/controller/internal/controller/lease_controller.go index eb5c7596f..0ba3ce0c0 100644 --- a/controller/internal/controller/lease_controller.go +++ b/controller/internal/controller/lease_controller.go @@ -25,6 +25,7 @@ import ( jumpstarterdevv1alpha1 "github.com/jumpstarter-dev/jumpstarter-controller/api/v1alpha1" corev1 "k8s.io/api/core/v1" + k8serrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" @@ -214,18 +215,48 @@ func (r *LeaseReconciler) reconcileStatusExporterRef( selector, err := lease.GetExporterSelector() if err != nil { return fmt.Errorf("reconcileStatusExporterRef: failed to get exporter selector: %w", err) - } else if selector.Empty() { + } else if selector.Empty() && lease.Spec.ExporterRef == nil { lease.SetStatusInvalid("InvalidSelector", "The selector for the lease is empty, a selector is required") return nil } - // List all Exporter matching selector - matchingExporters, err := r.ListMatchingExporters(ctx, lease, selector) - if err != nil { - return fmt.Errorf("reconcileStatusExporterRef: failed to list matching exporters: %w", err) + var matchingExporters []jumpstarterdevv1alpha1.Exporter + if lease.Spec.ExporterRef != nil { + var exporter jumpstarterdevv1alpha1.Exporter + if err := r.Get(ctx, types.NamespacedName{ + Namespace: lease.Namespace, + Name: lease.Spec.ExporterRef.Name, + }, &exporter); err != nil { + if k8serrors.IsNotFound(err) { + lease.SetStatusUnsatisfiable( + "ExporterNotFound", + "Requested exporter %s was not found", + lease.Spec.ExporterRef.Name, + ) + return nil + } + return fmt.Errorf("reconcileStatusExporterRef: failed to get requested exporter: %w", err) + } + if !selector.Empty() && !selector.Matches(labels.Set(exporter.Labels)) { + lease.SetStatusUnsatisfiable( + "SelectorMismatch", + "Requested exporter %s does not match selector %s", + exporter.Name, + metav1.FormatLabelSelector(&lease.Spec.Selector), + ) + return nil + } + matchingExporters = []jumpstarterdevv1alpha1.Exporter{exporter} + } else { + // List all exporters matching selector + listed, err := r.ListMatchingExporters(ctx, lease, selector) + if err != nil { + return fmt.Errorf("reconcileStatusExporterRef: failed to list matching exporters: %w", err) + } + matchingExporters = listed.Items } - approvedExporters, err := r.attachMatchingPolicies(ctx, lease, matchingExporters.Items) + approvedExporters, err := r.attachMatchingPolicies(ctx, lease, matchingExporters) if err != nil { return fmt.Errorf("reconcileStatusExporterRef: failed to handle policy approval: %w", err) } @@ -234,7 +265,7 @@ func (r *LeaseReconciler) reconcileStatusExporterRef( lease.SetStatusUnsatisfiable( "NoAccess", "While there are %d exporters matching the selector, none of them are approved by any policy for your client", - len(matchingExporters.Items), + len(matchingExporters), ) return nil } diff --git a/controller/internal/controller/lease_controller_test.go b/controller/internal/controller/lease_controller_test.go index 257ea1259..8694e89ae 100644 --- a/controller/internal/controller/lease_controller_test.go +++ b/controller/internal/controller/lease_controller_test.go @@ -77,20 +77,79 @@ var _ = Describe("Lease Controller", func() { lease := leaseDutA2Sec.DeepCopy() lease.Spec.Selector.MatchLabels = nil + ctx := context.Background() + err := k8sClient.Create(ctx, lease) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("one of selector or exporterRef.name is required")) + }) + }) + + When("trying to lease with a requested exporter and empty selector", func() { + It("should acquire the requested exporter", func() { + lease := leaseDutA2Sec.DeepCopy() + lease.Spec.Selector.MatchLabels = nil + lease.Spec.ExporterRef = &corev1.LocalObjectReference{Name: testExporter1DutA.Name} + ctx := context.Background() Expect(k8sClient.Create(ctx, lease)).To(Succeed()) _ = reconcileLease(ctx, lease) updatedLease := getLease(ctx, lease.Name) - Expect(updatedLease.Status.ExporterRef).To(BeNil()) - + Expect(updatedLease.Status.ExporterRef).NotTo(BeNil()) + Expect(updatedLease.Status.ExporterRef.Name).To(Equal(testExporter1DutA.Name)) Expect(meta.IsStatusConditionTrue( updatedLease.Status.Conditions, - string(jumpstarterdevv1alpha1.LeaseConditionTypeInvalid), + string(jumpstarterdevv1alpha1.LeaseConditionTypeReady), )).To(BeTrue()) }) }) + When("trying to lease with a missing requested exporter", func() { + It("should be unsatisfiable with ExporterNotFound reason", func() { + lease := leaseDutA2Sec.DeepCopy() + lease.Spec.Selector.MatchLabels = nil + lease.Spec.ExporterRef = &corev1.LocalObjectReference{Name: "does-not-exist"} + + ctx := context.Background() + Expect(k8sClient.Create(ctx, lease)).To(Succeed()) + _ = reconcileLease(ctx, lease) + + updatedLease := getLease(ctx, lease.Name) + Expect(updatedLease.Status.ExporterRef).To(BeNil()) + condition := meta.FindStatusCondition( + updatedLease.Status.Conditions, + string(jumpstarterdevv1alpha1.LeaseConditionTypeUnsatisfiable), + ) + Expect(condition).NotTo(BeNil()) + Expect(condition.Reason).To(Equal("ExporterNotFound")) + Expect(meta.IsStatusConditionTrue( + updatedLease.Status.Conditions, + string(jumpstarterdevv1alpha1.LeaseConditionTypePending), + )).To(BeFalse()) + }) + }) + + When("trying to lease with requested exporter that does not match selector", func() { + It("should fail with SelectorMismatch reason", func() { + lease := leaseDutA2Sec.DeepCopy() + lease.Spec.Selector.MatchLabels["dut"] = "b" + lease.Spec.ExporterRef = &corev1.LocalObjectReference{Name: testExporter1DutA.Name} + + ctx := context.Background() + Expect(k8sClient.Create(ctx, lease)).To(Succeed()) + _ = reconcileLease(ctx, lease) + + updatedLease := getLease(ctx, lease.Name) + Expect(updatedLease.Status.ExporterRef).To(BeNil()) + condition := meta.FindStatusCondition( + updatedLease.Status.Conditions, + string(jumpstarterdevv1alpha1.LeaseConditionTypeUnsatisfiable), + ) + Expect(condition).NotTo(BeNil()) + Expect(condition.Reason).To(Equal("SelectorMismatch")) + }) + }) + When("trying to lease an available exporter", func() { It("should acquire lease right away", func() { lease := leaseDutA2Sec.DeepCopy() diff --git a/controller/internal/protocol/jumpstarter/client/v1/client.pb.go b/controller/internal/protocol/jumpstarter/client/v1/client.pb.go index 19c21ddb6..e97c0119d 100644 --- a/controller/internal/protocol/jumpstarter/client/v1/client.pb.go +++ b/controller/internal/protocol/jumpstarter/client/v1/client.pb.go @@ -125,6 +125,7 @@ type Lease struct { Client *string `protobuf:"bytes,9,opt,name=client,proto3,oneof" json:"client,omitempty"` Exporter *string `protobuf:"bytes,10,opt,name=exporter,proto3,oneof" json:"exporter,omitempty"` Conditions []*v1.Condition `protobuf:"bytes,11,rep,name=conditions,proto3" json:"conditions,omitempty"` + ExporterName *string `protobuf:"bytes,12,opt,name=exporter_name,json=exporterName,proto3,oneof" json:"exporter_name,omitempty"` unknownFields protoimpl.UnknownFields sizeCache protoimpl.SizeCache } @@ -236,6 +237,13 @@ func (x *Lease) GetConditions() []*v1.Condition { return nil } +func (x *Lease) GetExporterName() string { + if x != nil && x.ExporterName != nil { + return *x.ExporterName + } + return "" +} + type GetExporterRequest struct { state protoimpl.MessageState `protogen:"open.v1"` Name string `protobuf:"bytes,1,opt,name=name,proto3" json:"name,omitempty"` @@ -742,7 +750,7 @@ const file_jumpstarter_client_v1_client_proto_rawDesc = "" + "\vLabelsEntry\x12\x10\n" + "\x03key\x18\x01 \x01(\tR\x03key\x12\x14\n" + "\x05value\x18\x02 \x01(\tR\x05value:\x028\x01:_\xeaA\\\n" + - "\x18jumpstarter.dev/Exporter\x12+namespaces/{namespace}/exporters/{exporter}*\texporters2\bexporter\"\xfa\x06\n" + + "\x18jumpstarter.dev/Exporter\x12+namespaces/{namespace}/exporters/{exporter}*\texporters2\bexporter\"\xbb\a\n" + "\x05Lease\x12\x17\n" + "\x04name\x18\x01 \x01(\tB\x03\xe0A\bR\x04name\x12\"\n" + "\bselector\x18\x02 \x01(\tB\x06\xe0A\x02\xe0A\x05R\bselector\x12:\n" + @@ -760,7 +768,8 @@ const file_jumpstarter_client_v1_client_proto_rawDesc = "" + "\x18jumpstarter.dev/ExporterH\x06R\bexporter\x88\x01\x01\x12>\n" + "\n" + "conditions\x18\v \x03(\v2\x19.jumpstarter.v1.ConditionB\x03\xe0A\x03R\n" + - "conditions:P\xeaAM\n" + + "conditions\x12-\n" + + "\rexporter_name\x18\f \x01(\tB\x03\xe0A\x05H\aR\fexporterName\x88\x01\x01:P\xeaAM\n" + "\x15jumpstarter.dev/Lease\x12%namespaces/{namespace}/leases/{lease}*\x06leases2\x05leaseB\v\n" + "\t_durationB\r\n" + "\v_begin_timeB\x17\n" + @@ -768,7 +777,8 @@ const file_jumpstarter_client_v1_client_proto_rawDesc = "" + "\t_end_timeB\x15\n" + "\x13_effective_end_timeB\t\n" + "\a_clientB\v\n" + - "\t_exporter\"J\n" + + "\t_exporterB\x10\n" + + "\x0e_exporter_name\"J\n" + "\x12GetExporterRequest\x124\n" + "\x04name\x18\x01 \x01(\tB \xe0A\x02\xfaA\x1a\n" + "\x18jumpstarter.dev/ExporterR\x04name\"\xb3\x01\n" + diff --git a/controller/internal/protocol/jumpstarter/client/v1/client_grpc.pb.go b/controller/internal/protocol/jumpstarter/client/v1/client_grpc.pb.go index 6055721cf..acac95b51 100644 --- a/controller/internal/protocol/jumpstarter/client/v1/client_grpc.pb.go +++ b/controller/internal/protocol/jumpstarter/client/v1/client_grpc.pb.go @@ -7,7 +7,7 @@ // Code generated by protoc-gen-go-grpc. DO NOT EDIT. // versions: -// - protoc-gen-go-grpc v1.6.0 +// - protoc-gen-go-grpc v1.6.1 // - protoc (unknown) // source: jumpstarter/client/v1/client.proto diff --git a/controller/internal/service/client/v1/client_service.go b/controller/internal/service/client/v1/client_service.go index a4e004566..9ea32bc85 100644 --- a/controller/internal/service/client/v1/client_service.go +++ b/controller/internal/service/client/v1/client_service.go @@ -25,6 +25,8 @@ import ( cpb "github.com/jumpstarter-dev/jumpstarter-controller/internal/protocol/jumpstarter/client/v1" "github.com/jumpstarter-dev/jumpstarter-controller/internal/service/auth" "github.com/jumpstarter-dev/jumpstarter-controller/internal/service/utils" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" "google.golang.org/protobuf/types/known/emptypb" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/labels" @@ -175,6 +177,14 @@ func (s *ClientService) ListLeases(ctx context.Context, req *cpb.ListLeasesReque } func (s *ClientService) CreateLease(ctx context.Context, req *cpb.CreateLeaseRequest) (*cpb.Lease, error) { + if req == nil { + return nil, status.Error(codes.InvalidArgument, "request is required") + } + + if err := validateLeaseTarget(req.Lease); err != nil { + return nil, err + } + namespace, err := utils.ParseNamespaceIdentifier(req.Parent) if err != nil { return nil, err @@ -212,6 +222,20 @@ func (s *ClientService) CreateLease(ctx context.Context, req *cpb.CreateLeaseReq return jlease.ToProtobuf(), nil } +func validateLeaseTarget(lease *cpb.Lease) error { + if lease == nil { + return status.Error(codes.InvalidArgument, "lease is required") + } + + hasSelector := lease.Selector != "" + hasExporterName := lease.ExporterName != nil && *lease.ExporterName != "" + if !hasSelector && !hasExporterName { + return status.Error(codes.InvalidArgument, "one of selector or exporter_name is required") + } + + return nil +} + func (s *ClientService) UpdateLease(ctx context.Context, req *cpb.UpdateLeaseRequest) (*cpb.Lease, error) { key, err := utils.ParseLeaseIdentifier(req.Lease.Name) if err != nil { diff --git a/controller/internal/service/client/v1/client_service_test.go b/controller/internal/service/client/v1/client_service_test.go new file mode 100644 index 000000000..52b869264 --- /dev/null +++ b/controller/internal/service/client/v1/client_service_test.go @@ -0,0 +1,88 @@ +package v1 + +import ( + "context" + "testing" + + cpb "github.com/jumpstarter-dev/jumpstarter-controller/internal/protocol/jumpstarter/client/v1" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" +) + +func TestValidateLeaseTarget(t *testing.T) { + t.Run("accepts selector target", func(t *testing.T) { + if err := validateLeaseTarget(&cpb.Lease{Selector: "dut=a"}); err != nil { + t.Fatalf("expected selector target to be valid, got error: %v", err) + } + }) + + t.Run("accepts exporter name target", func(t *testing.T) { + name := "laptop-test-exporter" + if err := validateLeaseTarget(&cpb.Lease{ExporterName: &name}); err != nil { + t.Fatalf("expected exporter name target to be valid, got error: %v", err) + } + }) + + t.Run("accepts selector and exporter name together", func(t *testing.T) { + name := "laptop-test-exporter" + if err := validateLeaseTarget(&cpb.Lease{Selector: "purpose=test", ExporterName: &name}); err != nil { + t.Fatalf("expected combined target to be valid, got error: %v", err) + } + }) + + t.Run("rejects missing selector and exporter name", func(t *testing.T) { + err := validateLeaseTarget(&cpb.Lease{}) + if err == nil { + t.Fatal("expected missing target to fail") + } + + st, ok := status.FromError(err) + if !ok { + t.Fatalf("expected grpc status error, got: %T", err) + } + if st.Code() != codes.InvalidArgument { + t.Fatalf("expected InvalidArgument, got: %v", st.Code()) + } + if st.Message() != "one of selector or exporter_name is required" { + t.Fatalf("unexpected message: %q", st.Message()) + } + }) + + t.Run("rejects nil lease", func(t *testing.T) { + err := validateLeaseTarget(nil) + if err == nil { + t.Fatal("expected nil lease to fail") + } + + st, ok := status.FromError(err) + if !ok { + t.Fatalf("expected grpc status error, got: %T", err) + } + if st.Code() != codes.InvalidArgument { + t.Fatalf("expected InvalidArgument, got: %v", st.Code()) + } + if st.Message() != "lease is required" { + t.Fatalf("unexpected message: %q", st.Message()) + } + }) +} + +func TestCreateLeaseRejectsNilRequest(t *testing.T) { + svc := &ClientService{} + + _, err := svc.CreateLease(context.Background(), nil) + if err == nil { + t.Fatal("expected nil request to fail") + } + + st, ok := status.FromError(err) + if !ok { + t.Fatalf("expected grpc status error, got: %T", err) + } + if st.Code() != codes.InvalidArgument { + t.Fatalf("expected InvalidArgument, got: %v", st.Code()) + } + if st.Message() != "request is required" { + t.Fatalf("unexpected message: %q", st.Message()) + } +} diff --git a/controller/internal/service/controller_service_integration_test.go b/controller/internal/service/controller_service_integration_test.go index 36b60d21e..ff4a6c75a 100644 --- a/controller/internal/service/controller_service_integration_test.go +++ b/controller/internal/service/controller_service_integration_test.go @@ -80,6 +80,7 @@ var _ = Describe("ControllerService Integration", func() { }, Spec: jumpstarterdevv1alpha1.LeaseSpec{ ClientRef: corev1.LocalObjectReference{Name: "test-client"}, + Selector: metav1.LabelSelector{MatchLabels: map[string]string{"test": "true"}}, Release: false, }, } @@ -125,6 +126,7 @@ var _ = Describe("ControllerService Integration", func() { }, Spec: jumpstarterdevv1alpha1.LeaseSpec{ ClientRef: corev1.LocalObjectReference{Name: "test-client"}, + Selector: metav1.LabelSelector{MatchLabels: map[string]string{"test": "true"}}, Release: true, // Already marked for release }, } @@ -162,6 +164,7 @@ var _ = Describe("ControllerService Integration", func() { }, Spec: jumpstarterdevv1alpha1.LeaseSpec{ ClientRef: corev1.LocalObjectReference{Name: "test-client"}, + Selector: metav1.LabelSelector{MatchLabels: map[string]string{"test": "true"}}, Release: false, }, } @@ -199,6 +202,7 @@ var _ = Describe("ControllerService Integration", func() { }, Spec: jumpstarterdevv1alpha1.LeaseSpec{ ClientRef: corev1.LocalObjectReference{Name: "test-client"}, + Selector: metav1.LabelSelector{MatchLabels: map[string]string{"test": "true"}}, }, } Expect(k8sClient.Create(ctx, lease)).To(Succeed()) @@ -236,6 +240,7 @@ var _ = Describe("ControllerService Integration", func() { }, Spec: jumpstarterdevv1alpha1.LeaseSpec{ ClientRef: corev1.LocalObjectReference{Name: "test-client"}, + Selector: metav1.LabelSelector{MatchLabels: map[string]string{"test": "true"}}, }, } Expect(k8sClient.Create(ctx, lease)).To(Succeed()) diff --git a/e2e/tests.bats b/e2e/tests.bats index 04608e554..0507a2763 100644 --- a/e2e/tests.bats +++ b/e2e/tests.bats @@ -353,6 +353,31 @@ EOF jmp shell --client test-client-oidc-provisioning --selector example.com/board=oidc j power on } +@test "can lease and connect to exporters by name" { + wait_for_exporter + + jmp shell --client test-client-oidc --name test-exporter-oidc j power on + jmp shell --client test-client-sa --name test-exporter-sa j power on + jmp shell --client test-client-legacy --name test-exporter-legacy j power on + + # Reusing the same exporter immediately can be flaky while it reconnects. + wait_for_exporter + + # --name and --selector together should work when they match. + jmp shell --client test-client-oidc --name test-exporter-oidc --selector example.com/board=oidc j power on +} + +@test "fails fast when requesting non-existent exporter by name" { + wait_for_exporter + + # Strict behavior: missing named exporter should become Unsatisfiable and fail quickly. + # If controller returns Pending here, this command will likely hit timeout (exit 124). + run timeout 20s jmp shell --client test-client-oidc --name test-exporter-does-not-exist j power on + assert_failure + [ "$status" -ne 124 ] + assert_output --partial "cannot be satisfied" +} + @test "can get crds with admin cli" { jmp admin get client --namespace "${JS_NAMESPACE}" jmp admin get exporter --namespace "${JS_NAMESPACE}" diff --git a/protocol/proto/jumpstarter/client/v1/client.proto b/protocol/proto/jumpstarter/client/v1/client.proto index 5cc5e8897..cc2acc037 100644 --- a/protocol/proto/jumpstarter/client/v1/client.proto +++ b/protocol/proto/jumpstarter/client/v1/client.proto @@ -102,6 +102,7 @@ message Lease { (google.api.resource_reference) = {type: "jumpstarter.dev/Exporter"} ]; repeated jumpstarter.v1.Condition conditions = 11 [(google.api.field_behavior) = OUTPUT_ONLY]; + optional string exporter_name = 12 [(google.api.field_behavior) = IMMUTABLE]; } message GetExporterRequest { diff --git a/python/packages/jumpstarter-cli/jumpstarter_cli/common.py b/python/packages/jumpstarter-cli/jumpstarter_cli/common.py index 52c522229..9316cec5f 100644 --- a/python/packages/jumpstarter-cli/jumpstarter_cli/common.py +++ b/python/packages/jumpstarter-cli/jumpstarter_cli/common.py @@ -19,6 +19,15 @@ def _opt_selector_callback(_ctx, _param, value): " Matching objects must satisfy all of the specified label constraints. Can be specified multiple times.", ) +opt_exporter_name = click.option( + "-n", + "--name", + "exporter_name", + type=str, + default=None, + help="Target a specific exporter/device name directly.", +) + class DurationParamType(click.ParamType): name = "duration" diff --git a/python/packages/jumpstarter-cli/jumpstarter_cli/create.py b/python/packages/jumpstarter-cli/jumpstarter_cli/create.py index c08598b5d..90969fb9c 100644 --- a/python/packages/jumpstarter-cli/jumpstarter_cli/create.py +++ b/python/packages/jumpstarter-cli/jumpstarter_cli/create.py @@ -6,7 +6,7 @@ from jumpstarter_cli_common.opt import OutputType, opt_output_all from jumpstarter_cli_common.print import model_print -from .common import opt_begin_time, opt_duration_partial, opt_selector +from .common import opt_begin_time, opt_duration_partial, opt_exporter_name, opt_selector from .login import relogin_client @@ -20,6 +20,7 @@ def create(): @create.command(name="lease") @opt_config(exporter=False) @opt_selector +@opt_exporter_name @opt_duration_partial(required=True) @opt_begin_time @click.option( @@ -31,7 +32,13 @@ def create(): @opt_output_all @handle_exceptions_with_reauthentication(relogin_client) def create_lease( - config, selector: str, duration: timedelta, begin_time: datetime | None, lease_id: str | None, output: OutputType + config, + selector: str | None, + exporter_name: str | None, + duration: timedelta, + begin_time: datetime | None, + lease_id: str | None, + output: OutputType, ): """ Create a lease @@ -64,6 +71,15 @@ def create_lease( """ - lease = config.create_lease(selector=selector, duration=duration, begin_time=begin_time, lease_id=lease_id) + if not selector and not exporter_name: + raise click.UsageError("one of --selector/-l or --name/-n is required") + + lease = config.create_lease( + selector=selector, + exporter_name=exporter_name, + duration=duration, + begin_time=begin_time, + lease_id=lease_id, + ) model_print(lease, output) diff --git a/python/packages/jumpstarter-cli/jumpstarter_cli/create_test.py b/python/packages/jumpstarter-cli/jumpstarter_cli/create_test.py new file mode 100644 index 000000000..b226a2813 --- /dev/null +++ b/python/packages/jumpstarter-cli/jumpstarter_cli/create_test.py @@ -0,0 +1,47 @@ +from datetime import timedelta +from unittest.mock import Mock, patch + +import click +import pytest + +from jumpstarter_cli.create import create_lease + + +def test_create_lease_passes_exporter_name_to_config(): + config = Mock() + lease = Mock() + config.create_lease.return_value = lease + + with patch("jumpstarter_cli.create.model_print") as model_print: + # Skip Click config loading wrapper and call the command body directly. + create_lease.callback.__wrapped__.__wrapped__( + config=config, + selector=None, + exporter_name="laptop-test-exporter", + duration=timedelta(minutes=5), + begin_time=None, + lease_id=None, + output="yaml", + ) + + config.create_lease.assert_called_once_with( + selector=None, + exporter_name="laptop-test-exporter", + duration=timedelta(minutes=5), + begin_time=None, + lease_id=None, + ) + model_print.assert_called_once_with(lease, "yaml") + + +def test_create_lease_requires_selector_or_name(): + with pytest.raises(click.UsageError, match="one of --selector/-l or --name/-n is required"): + create_lease.callback.__wrapped__.__wrapped__( + config=Mock(), + selector=None, + exporter_name=None, + duration=timedelta(minutes=5), + begin_time=None, + lease_id=None, + output="yaml", + ) diff --git a/python/packages/jumpstarter-cli/jumpstarter_cli/shell.py b/python/packages/jumpstarter-cli/jumpstarter_cli/shell.py index 155107906..55d20a91c 100644 --- a/python/packages/jumpstarter-cli/jumpstarter_cli/shell.py +++ b/python/packages/jumpstarter-cli/jumpstarter_cli/shell.py @@ -1,4 +1,5 @@ import logging +import os import sys from contextlib import ExitStack from datetime import timedelta @@ -15,13 +16,14 @@ ) from jumpstarter_cli_common.signal import signal_handler -from .common import opt_acquisition_timeout, opt_duration_partial, opt_selector +from .common import opt_acquisition_timeout, opt_duration_partial, opt_exporter_name, opt_selector from .login import relogin_client from jumpstarter.client.client import client_from_path from jumpstarter.common import HOOK_WARNING_PREFIX, ExporterStatus from jumpstarter.common.exceptions import ConnectionError, ExporterOfflineError from jumpstarter.common.utils import launch_shell from jumpstarter.config.client import ClientConfigV1Alpha1 +from jumpstarter.config.env import JMP_LEASE from jumpstarter.config.exporter import ExporterConfigV1Alpha1 logger = logging.getLogger(__name__) @@ -230,7 +232,7 @@ async def _run_shell_with_lease_async(lease, exporter_logs, config, command, can async def _shell_with_signal_handling( # noqa: C901 - config, selector, lease_name, duration, exporter_logs, command, acquisition_timeout + config, selector, exporter_name, lease_name, duration, exporter_logs, command, acquisition_timeout ): """Handle lease acquisition and shell execution with signal handling.""" exit_code = 0 @@ -250,7 +252,9 @@ async def _shell_with_signal_handling( # noqa: C901 try: try: async with anyio.from_thread.BlockingPortal() as portal: - async with config.lease_async(selector, lease_name, duration, portal, acquisition_timeout) as lease: + async with config.lease_async( + selector, exporter_name, lease_name, duration, portal, acquisition_timeout + ) as lease: lease_used = lease # Start token monitoring only once we're in the shell @@ -297,12 +301,22 @@ async def _shell_with_signal_handling( # noqa: C901 # TODO: warn if these are specified with exporter config @click.option("--lease", "lease_name") @opt_selector +@opt_exporter_name @opt_duration_partial(default=timedelta(minutes=30), show_default="00:30:00") @click.option("--exporter-logs", is_flag=True, help="Enable exporter log streaming") @opt_acquisition_timeout() # end client specific @handle_exceptions_with_reauthentication(relogin_client) -def shell(config, command: tuple[str, ...], lease_name, selector, duration, exporter_logs, acquisition_timeout): +def shell( + config, + command: tuple[str, ...], + lease_name, + selector, + exporter_name, + duration, + exporter_logs, + acquisition_timeout, +): """ Spawns a shell (or custom command) connecting to a local or remote exporter @@ -317,10 +331,14 @@ def shell(config, command: tuple[str, ...], lease_name, selector, duration, expo match config: case ClientConfigV1Alpha1(): + has_existing_lease = bool(lease_name or os.environ.get(JMP_LEASE)) + if not selector and not exporter_name and not has_existing_lease: + raise click.UsageError("one of --selector/-l or --name/-n is required") exit_code = anyio.run( _shell_with_signal_handling, config, selector, + exporter_name, lease_name, duration, exporter_logs, diff --git a/python/packages/jumpstarter-cli/jumpstarter_cli/shell_test.py b/python/packages/jumpstarter-cli/jumpstarter_cli/shell_test.py new file mode 100644 index 000000000..ceb3a225f --- /dev/null +++ b/python/packages/jumpstarter-cli/jumpstarter_cli/shell_test.py @@ -0,0 +1,100 @@ +from contextlib import asynccontextmanager +from datetime import timedelta +from unittest.mock import AsyncMock, Mock, patch + +import anyio +import click +import pytest + +from jumpstarter_cli.shell import _shell_with_signal_handling, shell + +from jumpstarter.config.client import ClientConfigV1Alpha1 +from jumpstarter.config.env import JMP_LEASE + + +class _DummyConfig: + def __init__(self): + self.captured = None + self.token = None + + @asynccontextmanager + async def lease_async(self, selector, exporter_name, lease_name, duration, portal, acquisition_timeout): + self.captured = (selector, exporter_name, lease_name, duration, acquisition_timeout) + yield Mock() + + +def test_shell_passes_exporter_name_to_lease_async(): + config = _DummyConfig() + + with patch( + "jumpstarter_cli.shell._run_shell_with_lease_async", + new=AsyncMock(return_value=0), + ): + exit_code = anyio.run( + _shell_with_signal_handling, + config, + None, + "laptop-test-exporter", + None, + timedelta(minutes=1), + False, + (), + None, + ) + + assert exit_code == 0 + assert config.captured is not None + assert config.captured[1] == "laptop-test-exporter" + + +def test_shell_requires_selector_or_name(): + with pytest.raises(click.UsageError, match="one of --selector/-l or --name/-n is required"): + shell.callback.__wrapped__.__wrapped__( + config=Mock(spec=ClientConfigV1Alpha1), + command=(), + lease_name=None, + selector=None, + exporter_name=None, + duration=timedelta(minutes=1), + exporter_logs=False, + acquisition_timeout=None, + ) + + +def test_shell_allows_existing_lease_name_without_selector_or_name(): + with ( + patch("jumpstarter_cli.shell.anyio.run", return_value=0), + patch("jumpstarter_cli.shell.sys.exit") as mock_exit, + ): + shell.callback.__wrapped__.__wrapped__( + config=Mock(spec=ClientConfigV1Alpha1), + command=(), + lease_name="existing-lease", + selector=None, + exporter_name=None, + duration=timedelta(minutes=1), + exporter_logs=False, + acquisition_timeout=None, + ) + + mock_exit.assert_called_once_with(0) + + +def test_shell_allows_env_lease_without_selector_or_name(): + with ( + patch("jumpstarter_cli.shell.anyio.run", return_value=0), + patch("jumpstarter_cli.shell.sys.exit") as mock_exit, + patch.dict("os.environ", {JMP_LEASE: "existing-lease"}, clear=False), + ): + shell.callback.__wrapped__.__wrapped__( + config=Mock(spec=ClientConfigV1Alpha1), + command=(), + lease_name=None, + selector=None, + exporter_name=None, + duration=timedelta(minutes=1), + exporter_logs=False, + acquisition_timeout=None, + ) + + mock_exit.assert_called_once_with(0) diff --git a/python/packages/jumpstarter-kubernetes/jumpstarter_kubernetes/leases.py b/python/packages/jumpstarter-kubernetes/jumpstarter_kubernetes/leases.py index ca0019fd8..e371a1c00 100644 --- a/python/packages/jumpstarter-kubernetes/jumpstarter_kubernetes/leases.py +++ b/python/packages/jumpstarter-kubernetes/jumpstarter_kubernetes/leases.py @@ -37,6 +37,7 @@ class V1Alpha1Lease(JsonBaseModel): @staticmethod def from_dict(dict: dict): + selector_data = dict["spec"].get("selector", {}) return V1Alpha1Lease( api_version=dict["apiVersion"], kind=dict["kind"], @@ -73,7 +74,7 @@ def from_dict(dict: dict): if "clientRef" in dict["spec"] else None, duration=dict["spec"]["duration"] if "duration" in dict["spec"] else None, - selector=V1Alpha1LeaseSelector(match_labels=dict["spec"]["selector"]["matchLabels"]), + selector=V1Alpha1LeaseSelector(match_labels=selector_data.get("matchLabels", {})), ), ) diff --git a/python/packages/jumpstarter-kubernetes/jumpstarter_kubernetes/test_leases.py b/python/packages/jumpstarter-kubernetes/jumpstarter_kubernetes/test_leases.py index d7a45cb5c..476ce4e60 100644 --- a/python/packages/jumpstarter-kubernetes/jumpstarter_kubernetes/test_leases.py +++ b/python/packages/jumpstarter-kubernetes/jumpstarter_kubernetes/test_leases.py @@ -113,3 +113,42 @@ def test_lease_dump_yaml(): name: test-exporter """ ) + + +def test_lease_from_dict_without_match_labels_name_targeted(): + lease = V1Alpha1Lease.from_dict( + { + "apiVersion": "jumpstarter.dev/v1alpha1", + "kind": "Lease", + "metadata": { + "creationTimestamp": "2021-10-01T00:00:00Z", + "generation": 1, + "managedFields": [], + "name": "test-lease", + "namespace": "default", + "resourceVersion": "1", + "uid": "7a25eb81-6443-47ec-a62f-50165bffede8", + }, + "spec": { + "clientRef": {"name": "test-client"}, + "duration": "1h", + "selector": {}, + "exporterRef": {"name": "test-exporter"}, + }, + "status": { + "ended": False, + "conditions": [ + { + "lastTransitionTime": "2021-10-01T00:00:00Z", + "message": "", + "observedGeneration": 1, + "reason": "", + "status": "True", + "type": "Active", + } + ], + }, + } + ) + + assert lease.spec.selector.match_labels == {} diff --git a/python/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/client/v1/client_pb2.py b/python/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/client/v1/client_pb2.py index cd9f2c93b..19ee1977c 100644 --- a/python/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/client/v1/client_pb2.py +++ b/python/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/client/v1/client_pb2.py @@ -34,7 +34,7 @@ from ...v1 import common_pb2 as jumpstarter_dot_v1_dot_common__pb2 -DESCRIPTOR = _descriptor_pool.Default().AddSerializedFile(b'\n\"jumpstarter/client/v1/client.proto\x12\x15jumpstarter.client.v1\x1a\x1cgoogle/api/annotations.proto\x1a\x17google/api/client.proto\x1a\x1fgoogle/api/field_behavior.proto\x1a\x19google/api/resource.proto\x1a\x1egoogle/protobuf/duration.proto\x1a\x1bgoogle/protobuf/empty.proto\x1a google/protobuf/field_mask.proto\x1a\x1fgoogle/protobuf/timestamp.proto\x1a\x1fjumpstarter/v1/kubernetes.proto\x1a\x1bjumpstarter/v1/common.proto\"\x8c\x03\n\x08\x45xporter\x12\x17\n\x04name\x18\x01 \x01(\tB\x03\xe0\x41\x08R\x04name\x12\x43\n\x06labels\x18\x02 \x03(\x0b\x32+.jumpstarter.client.v1.Exporter.LabelsEntryR\x06labels\x12\x1d\n\x06online\x18\x03 \x01(\x08\x42\x05\x18\x01\xe0\x41\x03R\x06online\x12;\n\x06status\x18\x04 \x01(\x0e\x32\x1e.jumpstarter.v1.ExporterStatusB\x03\xe0\x41\x03R\x06status\x12*\n\x0estatus_message\x18\x05 \x01(\tB\x03\xe0\x41\x03R\rstatusMessage\x1a\x39\n\x0bLabelsEntry\x12\x10\n\x03key\x18\x01 \x01(\tR\x03key\x12\x14\n\x05value\x18\x02 \x01(\tR\x05value:\x02\x38\x01:_\xea\x41\\\n\x18jumpstarter.dev/Exporter\x12+namespaces/{namespace}/exporters/{exporter}*\texporters2\x08\x65xporter\"\xfa\x06\n\x05Lease\x12\x17\n\x04name\x18\x01 \x01(\tB\x03\xe0\x41\x08R\x04name\x12\"\n\x08selector\x18\x02 \x01(\tB\x06\xe0\x41\x02\xe0\x41\x05R\x08selector\x12:\n\x08\x64uration\x18\x03 \x01(\x0b\x32\x19.google.protobuf.DurationH\x00R\x08\x64uration\x88\x01\x01\x12M\n\x12\x65\x66\x66\x65\x63tive_duration\x18\x04 \x01(\x0b\x32\x19.google.protobuf.DurationB\x03\xe0\x41\x03R\x11\x65\x66\x66\x65\x63tiveDuration\x12>\n\nbegin_time\x18\x05 \x01(\x0b\x32\x1a.google.protobuf.TimestampH\x01R\tbeginTime\x88\x01\x01\x12V\n\x14\x65\x66\x66\x65\x63tive_begin_time\x18\x06 \x01(\x0b\x32\x1a.google.protobuf.TimestampB\x03\xe0\x41\x03H\x02R\x12\x65\x66\x66\x65\x63tiveBeginTime\x88\x01\x01\x12:\n\x08\x65nd_time\x18\x07 \x01(\x0b\x32\x1a.google.protobuf.TimestampH\x03R\x07\x65ndTime\x88\x01\x01\x12R\n\x12\x65\x66\x66\x65\x63tive_end_time\x18\x08 \x01(\x0b\x32\x1a.google.protobuf.TimestampB\x03\xe0\x41\x03H\x04R\x10\x65\x66\x66\x65\x63tiveEndTime\x88\x01\x01\x12;\n\x06\x63lient\x18\t \x01(\tB\x1e\xe0\x41\x03\xfa\x41\x18\n\x16jumpstarter.dev/ClientH\x05R\x06\x63lient\x88\x01\x01\x12\x41\n\x08\x65xporter\x18\n \x01(\tB \xe0\x41\x03\xfa\x41\x1a\n\x18jumpstarter.dev/ExporterH\x06R\x08\x65xporter\x88\x01\x01\x12>\n\nconditions\x18\x0b \x03(\x0b\x32\x19.jumpstarter.v1.ConditionB\x03\xe0\x41\x03R\nconditions:P\xea\x41M\n\x15jumpstarter.dev/Lease\x12%namespaces/{namespace}/leases/{lease}*\x06leases2\x05leaseB\x0b\n\t_durationB\r\n\x0b_begin_timeB\x17\n\x15_effective_begin_timeB\x0b\n\t_end_timeB\x15\n\x13_effective_end_timeB\t\n\x07_clientB\x0b\n\t_exporter\"J\n\x12GetExporterRequest\x12\x34\n\x04name\x18\x01 \x01(\tB \xe0\x41\x02\xfa\x41\x1a\n\x18jumpstarter.dev/ExporterR\x04name\"\xb3\x01\n\x14ListExportersRequest\x12\x38\n\x06parent\x18\x01 \x01(\tB \xe0\x41\x02\xfa\x41\x1a\x12\x18jumpstarter.dev/ExporterR\x06parent\x12 \n\tpage_size\x18\x02 \x01(\x05\x42\x03\xe0\x41\x01R\x08pageSize\x12\"\n\npage_token\x18\x03 \x01(\tB\x03\xe0\x41\x01R\tpageToken\x12\x1b\n\x06\x66ilter\x18\x04 \x01(\tB\x03\xe0\x41\x01R\x06\x66ilter\"~\n\x15ListExportersResponse\x12=\n\texporters\x18\x01 \x03(\x0b\x32\x1f.jumpstarter.client.v1.ExporterR\texporters\x12&\n\x0fnext_page_token\x18\x02 \x01(\tR\rnextPageToken\"D\n\x0fGetLeaseRequest\x12\x31\n\x04name\x18\x01 \x01(\tB\x1d\xe0\x41\x02\xfa\x41\x17\n\x15jumpstarter.dev/LeaseR\x04name\"\xe8\x01\n\x11ListLeasesRequest\x12\x35\n\x06parent\x18\x01 \x01(\tB\x1d\xe0\x41\x02\xfa\x41\x17\x12\x15jumpstarter.dev/LeaseR\x06parent\x12 \n\tpage_size\x18\x02 \x01(\x05\x42\x03\xe0\x41\x01R\x08pageSize\x12\"\n\npage_token\x18\x03 \x01(\tB\x03\xe0\x41\x01R\tpageToken\x12\x1b\n\x06\x66ilter\x18\x04 \x01(\tB\x03\xe0\x41\x01R\x06\x66ilter\x12)\n\x0bonly_active\x18\x05 \x01(\x08\x42\x03\xe0\x41\x01H\x00R\nonlyActive\x88\x01\x01\x42\x0e\n\x0c_only_active\"r\n\x12ListLeasesResponse\x12\x34\n\x06leases\x18\x01 \x03(\x0b\x32\x1c.jumpstarter.client.v1.LeaseR\x06leases\x12&\n\x0fnext_page_token\x18\x02 \x01(\tR\rnextPageToken\"\xa4\x01\n\x12\x43reateLeaseRequest\x12\x35\n\x06parent\x18\x01 \x01(\tB\x1d\xe0\x41\x02\xfa\x41\x17\x12\x15jumpstarter.dev/LeaseR\x06parent\x12\x1e\n\x08lease_id\x18\x02 \x01(\tB\x03\xe0\x41\x01R\x07leaseId\x12\x37\n\x05lease\x18\x03 \x01(\x0b\x32\x1c.jumpstarter.client.v1.LeaseB\x03\xe0\x41\x02R\x05lease\"\x8f\x01\n\x12UpdateLeaseRequest\x12\x37\n\x05lease\x18\x01 \x01(\x0b\x32\x1c.jumpstarter.client.v1.LeaseB\x03\xe0\x41\x02R\x05lease\x12@\n\x0bupdate_mask\x18\x02 \x01(\x0b\x32\x1a.google.protobuf.FieldMaskB\x03\xe0\x41\x01R\nupdateMask\"G\n\x12\x44\x65leteLeaseRequest\x12\x31\n\x04name\x18\x01 \x01(\tB\x1d\xe0\x41\x02\xfa\x41\x17\n\x15jumpstarter.dev/LeaseR\x04name2\xa7\x08\n\rClientService\x12\x8d\x01\n\x0bGetExporter\x12).jumpstarter.client.v1.GetExporterRequest\x1a\x1f.jumpstarter.client.v1.Exporter\"2\xda\x41\x04name\x82\xd3\xe4\x93\x02%\x12#/v1/{name=namespaces/*/exporters/*}\x12\xa0\x01\n\rListExporters\x12+.jumpstarter.client.v1.ListExportersRequest\x1a,.jumpstarter.client.v1.ListExportersResponse\"4\xda\x41\x06parent\x82\xd3\xe4\x93\x02%\x12#/v1/{parent=namespaces/*}/exporters\x12\x81\x01\n\x08GetLease\x12&.jumpstarter.client.v1.GetLeaseRequest\x1a\x1c.jumpstarter.client.v1.Lease\"/\xda\x41\x04name\x82\xd3\xe4\x93\x02\"\x12 /v1/{name=namespaces/*/leases/*}\x12\x94\x01\n\nListLeases\x12(.jumpstarter.client.v1.ListLeasesRequest\x1a).jumpstarter.client.v1.ListLeasesResponse\"1\xda\x41\x06parent\x82\xd3\xe4\x93\x02\"\x12 /v1/{parent=namespaces/*}/leases\x12\x9f\x01\n\x0b\x43reateLease\x12).jumpstarter.client.v1.CreateLeaseRequest\x1a\x1c.jumpstarter.client.v1.Lease\"G\xda\x41\x15parent,lease,lease_id\x82\xd3\xe4\x93\x02)\" /v1/{parent=namespaces/*}/leases:\x05lease\x12\xa1\x01\n\x0bUpdateLease\x12).jumpstarter.client.v1.UpdateLeaseRequest\x1a\x1c.jumpstarter.client.v1.Lease\"I\xda\x41\x11lease,update_mask\x82\xd3\xe4\x93\x02/2&/v1/{lease.name=namespaces/*/leases/*}:\x05lease\x12\x81\x01\n\x0b\x44\x65leteLease\x12).jumpstarter.client.v1.DeleteLeaseRequest\x1a\x16.google.protobuf.Empty\"/\xda\x41\x04name\x82\xd3\xe4\x93\x02\"* /v1/{name=namespaces/*/leases/*}B\x9e\x01\n\x19\x63om.jumpstarter.client.v1B\x0b\x43lientProtoP\x01\xa2\x02\x03JCX\xaa\x02\x15Jumpstarter.Client.V1\xca\x02\x15Jumpstarter\\Client\\V1\xe2\x02!Jumpstarter\\Client\\V1\\GPBMetadata\xea\x02\x17Jumpstarter::Client::V1b\x06proto3') +DESCRIPTOR = _descriptor_pool.Default().AddSerializedFile(b'\n\"jumpstarter/client/v1/client.proto\x12\x15jumpstarter.client.v1\x1a\x1cgoogle/api/annotations.proto\x1a\x17google/api/client.proto\x1a\x1fgoogle/api/field_behavior.proto\x1a\x19google/api/resource.proto\x1a\x1egoogle/protobuf/duration.proto\x1a\x1bgoogle/protobuf/empty.proto\x1a google/protobuf/field_mask.proto\x1a\x1fgoogle/protobuf/timestamp.proto\x1a\x1fjumpstarter/v1/kubernetes.proto\x1a\x1bjumpstarter/v1/common.proto\"\x8c\x03\n\x08\x45xporter\x12\x17\n\x04name\x18\x01 \x01(\tB\x03\xe0\x41\x08R\x04name\x12\x43\n\x06labels\x18\x02 \x03(\x0b\x32+.jumpstarter.client.v1.Exporter.LabelsEntryR\x06labels\x12\x1d\n\x06online\x18\x03 \x01(\x08\x42\x05\x18\x01\xe0\x41\x03R\x06online\x12;\n\x06status\x18\x04 \x01(\x0e\x32\x1e.jumpstarter.v1.ExporterStatusB\x03\xe0\x41\x03R\x06status\x12*\n\x0estatus_message\x18\x05 \x01(\tB\x03\xe0\x41\x03R\rstatusMessage\x1a\x39\n\x0bLabelsEntry\x12\x10\n\x03key\x18\x01 \x01(\tR\x03key\x12\x14\n\x05value\x18\x02 \x01(\tR\x05value:\x02\x38\x01:_\xea\x41\\\n\x18jumpstarter.dev/Exporter\x12+namespaces/{namespace}/exporters/{exporter}*\texporters2\x08\x65xporter\"\xbb\x07\n\x05Lease\x12\x17\n\x04name\x18\x01 \x01(\tB\x03\xe0\x41\x08R\x04name\x12\"\n\x08selector\x18\x02 \x01(\tB\x06\xe0\x41\x02\xe0\x41\x05R\x08selector\x12:\n\x08\x64uration\x18\x03 \x01(\x0b\x32\x19.google.protobuf.DurationH\x00R\x08\x64uration\x88\x01\x01\x12M\n\x12\x65\x66\x66\x65\x63tive_duration\x18\x04 \x01(\x0b\x32\x19.google.protobuf.DurationB\x03\xe0\x41\x03R\x11\x65\x66\x66\x65\x63tiveDuration\x12>\n\nbegin_time\x18\x05 \x01(\x0b\x32\x1a.google.protobuf.TimestampH\x01R\tbeginTime\x88\x01\x01\x12V\n\x14\x65\x66\x66\x65\x63tive_begin_time\x18\x06 \x01(\x0b\x32\x1a.google.protobuf.TimestampB\x03\xe0\x41\x03H\x02R\x12\x65\x66\x66\x65\x63tiveBeginTime\x88\x01\x01\x12:\n\x08\x65nd_time\x18\x07 \x01(\x0b\x32\x1a.google.protobuf.TimestampH\x03R\x07\x65ndTime\x88\x01\x01\x12R\n\x12\x65\x66\x66\x65\x63tive_end_time\x18\x08 \x01(\x0b\x32\x1a.google.protobuf.TimestampB\x03\xe0\x41\x03H\x04R\x10\x65\x66\x66\x65\x63tiveEndTime\x88\x01\x01\x12;\n\x06\x63lient\x18\t \x01(\tB\x1e\xe0\x41\x03\xfa\x41\x18\n\x16jumpstarter.dev/ClientH\x05R\x06\x63lient\x88\x01\x01\x12\x41\n\x08\x65xporter\x18\n \x01(\tB \xe0\x41\x03\xfa\x41\x1a\n\x18jumpstarter.dev/ExporterH\x06R\x08\x65xporter\x88\x01\x01\x12>\n\nconditions\x18\x0b \x03(\x0b\x32\x19.jumpstarter.v1.ConditionB\x03\xe0\x41\x03R\nconditions\x12-\n\rexporter_name\x18\x0c \x01(\tB\x03\xe0\x41\x05H\x07R\x0c\x65xporterName\x88\x01\x01:P\xea\x41M\n\x15jumpstarter.dev/Lease\x12%namespaces/{namespace}/leases/{lease}*\x06leases2\x05leaseB\x0b\n\t_durationB\r\n\x0b_begin_timeB\x17\n\x15_effective_begin_timeB\x0b\n\t_end_timeB\x15\n\x13_effective_end_timeB\t\n\x07_clientB\x0b\n\t_exporterB\x10\n\x0e_exporter_name\"J\n\x12GetExporterRequest\x12\x34\n\x04name\x18\x01 \x01(\tB \xe0\x41\x02\xfa\x41\x1a\n\x18jumpstarter.dev/ExporterR\x04name\"\xb3\x01\n\x14ListExportersRequest\x12\x38\n\x06parent\x18\x01 \x01(\tB \xe0\x41\x02\xfa\x41\x1a\x12\x18jumpstarter.dev/ExporterR\x06parent\x12 \n\tpage_size\x18\x02 \x01(\x05\x42\x03\xe0\x41\x01R\x08pageSize\x12\"\n\npage_token\x18\x03 \x01(\tB\x03\xe0\x41\x01R\tpageToken\x12\x1b\n\x06\x66ilter\x18\x04 \x01(\tB\x03\xe0\x41\x01R\x06\x66ilter\"~\n\x15ListExportersResponse\x12=\n\texporters\x18\x01 \x03(\x0b\x32\x1f.jumpstarter.client.v1.ExporterR\texporters\x12&\n\x0fnext_page_token\x18\x02 \x01(\tR\rnextPageToken\"D\n\x0fGetLeaseRequest\x12\x31\n\x04name\x18\x01 \x01(\tB\x1d\xe0\x41\x02\xfa\x41\x17\n\x15jumpstarter.dev/LeaseR\x04name\"\xe8\x01\n\x11ListLeasesRequest\x12\x35\n\x06parent\x18\x01 \x01(\tB\x1d\xe0\x41\x02\xfa\x41\x17\x12\x15jumpstarter.dev/LeaseR\x06parent\x12 \n\tpage_size\x18\x02 \x01(\x05\x42\x03\xe0\x41\x01R\x08pageSize\x12\"\n\npage_token\x18\x03 \x01(\tB\x03\xe0\x41\x01R\tpageToken\x12\x1b\n\x06\x66ilter\x18\x04 \x01(\tB\x03\xe0\x41\x01R\x06\x66ilter\x12)\n\x0bonly_active\x18\x05 \x01(\x08\x42\x03\xe0\x41\x01H\x00R\nonlyActive\x88\x01\x01\x42\x0e\n\x0c_only_active\"r\n\x12ListLeasesResponse\x12\x34\n\x06leases\x18\x01 \x03(\x0b\x32\x1c.jumpstarter.client.v1.LeaseR\x06leases\x12&\n\x0fnext_page_token\x18\x02 \x01(\tR\rnextPageToken\"\xa4\x01\n\x12\x43reateLeaseRequest\x12\x35\n\x06parent\x18\x01 \x01(\tB\x1d\xe0\x41\x02\xfa\x41\x17\x12\x15jumpstarter.dev/LeaseR\x06parent\x12\x1e\n\x08lease_id\x18\x02 \x01(\tB\x03\xe0\x41\x01R\x07leaseId\x12\x37\n\x05lease\x18\x03 \x01(\x0b\x32\x1c.jumpstarter.client.v1.LeaseB\x03\xe0\x41\x02R\x05lease\"\x8f\x01\n\x12UpdateLeaseRequest\x12\x37\n\x05lease\x18\x01 \x01(\x0b\x32\x1c.jumpstarter.client.v1.LeaseB\x03\xe0\x41\x02R\x05lease\x12@\n\x0bupdate_mask\x18\x02 \x01(\x0b\x32\x1a.google.protobuf.FieldMaskB\x03\xe0\x41\x01R\nupdateMask\"G\n\x12\x44\x65leteLeaseRequest\x12\x31\n\x04name\x18\x01 \x01(\tB\x1d\xe0\x41\x02\xfa\x41\x17\n\x15jumpstarter.dev/LeaseR\x04name2\xa7\x08\n\rClientService\x12\x8d\x01\n\x0bGetExporter\x12).jumpstarter.client.v1.GetExporterRequest\x1a\x1f.jumpstarter.client.v1.Exporter\"2\xda\x41\x04name\x82\xd3\xe4\x93\x02%\x12#/v1/{name=namespaces/*/exporters/*}\x12\xa0\x01\n\rListExporters\x12+.jumpstarter.client.v1.ListExportersRequest\x1a,.jumpstarter.client.v1.ListExportersResponse\"4\xda\x41\x06parent\x82\xd3\xe4\x93\x02%\x12#/v1/{parent=namespaces/*}/exporters\x12\x81\x01\n\x08GetLease\x12&.jumpstarter.client.v1.GetLeaseRequest\x1a\x1c.jumpstarter.client.v1.Lease\"/\xda\x41\x04name\x82\xd3\xe4\x93\x02\"\x12 /v1/{name=namespaces/*/leases/*}\x12\x94\x01\n\nListLeases\x12(.jumpstarter.client.v1.ListLeasesRequest\x1a).jumpstarter.client.v1.ListLeasesResponse\"1\xda\x41\x06parent\x82\xd3\xe4\x93\x02\"\x12 /v1/{parent=namespaces/*}/leases\x12\x9f\x01\n\x0b\x43reateLease\x12).jumpstarter.client.v1.CreateLeaseRequest\x1a\x1c.jumpstarter.client.v1.Lease\"G\xda\x41\x15parent,lease,lease_id\x82\xd3\xe4\x93\x02)\" /v1/{parent=namespaces/*}/leases:\x05lease\x12\xa1\x01\n\x0bUpdateLease\x12).jumpstarter.client.v1.UpdateLeaseRequest\x1a\x1c.jumpstarter.client.v1.Lease\"I\xda\x41\x11lease,update_mask\x82\xd3\xe4\x93\x02/2&/v1/{lease.name=namespaces/*/leases/*}:\x05lease\x12\x81\x01\n\x0b\x44\x65leteLease\x12).jumpstarter.client.v1.DeleteLeaseRequest\x1a\x16.google.protobuf.Empty\"/\xda\x41\x04name\x82\xd3\xe4\x93\x02\"* /v1/{name=namespaces/*/leases/*}B\x9e\x01\n\x19\x63om.jumpstarter.client.v1B\x0b\x43lientProtoP\x01\xa2\x02\x03JCX\xaa\x02\x15Jumpstarter.Client.V1\xca\x02\x15Jumpstarter\\Client\\V1\xe2\x02!Jumpstarter\\Client\\V1\\GPBMetadata\xea\x02\x17Jumpstarter::Client::V1b\x06proto3') _globals = globals() _builder.BuildMessageAndEnumDescriptors(DESCRIPTOR, _globals) @@ -70,6 +70,8 @@ _globals['_LEASE'].fields_by_name['exporter']._serialized_options = b'\340A\003\372A\032\n\030jumpstarter.dev/Exporter' _globals['_LEASE'].fields_by_name['conditions']._loaded_options = None _globals['_LEASE'].fields_by_name['conditions']._serialized_options = b'\340A\003' + _globals['_LEASE'].fields_by_name['exporter_name']._loaded_options = None + _globals['_LEASE'].fields_by_name['exporter_name']._serialized_options = b'\340A\005' _globals['_LEASE']._loaded_options = None _globals['_LEASE']._serialized_options = b'\352AM\n\025jumpstarter.dev/Lease\022%namespaces/{namespace}/leases/{lease}*\006leases2\005lease' _globals['_GETEXPORTERREQUEST'].fields_by_name['name']._loaded_options = None @@ -125,25 +127,25 @@ _globals['_EXPORTER_LABELSENTRY']._serialized_start=609 _globals['_EXPORTER_LABELSENTRY']._serialized_end=666 _globals['_LEASE']._serialized_start=766 - _globals['_LEASE']._serialized_end=1656 - _globals['_GETEXPORTERREQUEST']._serialized_start=1658 - _globals['_GETEXPORTERREQUEST']._serialized_end=1732 - _globals['_LISTEXPORTERSREQUEST']._serialized_start=1735 - _globals['_LISTEXPORTERSREQUEST']._serialized_end=1914 - _globals['_LISTEXPORTERSRESPONSE']._serialized_start=1916 - _globals['_LISTEXPORTERSRESPONSE']._serialized_end=2042 - _globals['_GETLEASEREQUEST']._serialized_start=2044 - _globals['_GETLEASEREQUEST']._serialized_end=2112 - _globals['_LISTLEASESREQUEST']._serialized_start=2115 - _globals['_LISTLEASESREQUEST']._serialized_end=2347 - _globals['_LISTLEASESRESPONSE']._serialized_start=2349 - _globals['_LISTLEASESRESPONSE']._serialized_end=2463 - _globals['_CREATELEASEREQUEST']._serialized_start=2466 - _globals['_CREATELEASEREQUEST']._serialized_end=2630 - _globals['_UPDATELEASEREQUEST']._serialized_start=2633 - _globals['_UPDATELEASEREQUEST']._serialized_end=2776 - _globals['_DELETELEASEREQUEST']._serialized_start=2778 - _globals['_DELETELEASEREQUEST']._serialized_end=2849 - _globals['_CLIENTSERVICE']._serialized_start=2852 - _globals['_CLIENTSERVICE']._serialized_end=3915 + _globals['_LEASE']._serialized_end=1721 + _globals['_GETEXPORTERREQUEST']._serialized_start=1723 + _globals['_GETEXPORTERREQUEST']._serialized_end=1797 + _globals['_LISTEXPORTERSREQUEST']._serialized_start=1800 + _globals['_LISTEXPORTERSREQUEST']._serialized_end=1979 + _globals['_LISTEXPORTERSRESPONSE']._serialized_start=1981 + _globals['_LISTEXPORTERSRESPONSE']._serialized_end=2107 + _globals['_GETLEASEREQUEST']._serialized_start=2109 + _globals['_GETLEASEREQUEST']._serialized_end=2177 + _globals['_LISTLEASESREQUEST']._serialized_start=2180 + _globals['_LISTLEASESREQUEST']._serialized_end=2412 + _globals['_LISTLEASESRESPONSE']._serialized_start=2414 + _globals['_LISTLEASESRESPONSE']._serialized_end=2528 + _globals['_CREATELEASEREQUEST']._serialized_start=2531 + _globals['_CREATELEASEREQUEST']._serialized_end=2695 + _globals['_UPDATELEASEREQUEST']._serialized_start=2698 + _globals['_UPDATELEASEREQUEST']._serialized_end=2841 + _globals['_DELETELEASEREQUEST']._serialized_start=2843 + _globals['_DELETELEASEREQUEST']._serialized_end=2914 + _globals['_CLIENTSERVICE']._serialized_start=2917 + _globals['_CLIENTSERVICE']._serialized_end=3980 # @@protoc_insertion_point(module_scope) diff --git a/python/packages/jumpstarter/jumpstarter/client/grpc.py b/python/packages/jumpstarter/jumpstarter/client/grpc.py index ee69a7def..d0fea0dc0 100644 --- a/python/packages/jumpstarter/jumpstarter/client/grpc.py +++ b/python/packages/jumpstarter/jumpstarter/client/grpc.py @@ -133,6 +133,7 @@ class Lease(BaseModel): namespace: str name: str selector: str + exporter_name: str | None = None duration: timedelta effective_duration: timedelta | None = None begin_time: datetime | None = None @@ -187,6 +188,7 @@ def from_protobuf(cls, data: client_pb2.Lease) -> Lease: namespace=namespace, name=name, selector=data.selector, + exporter_name=data.exporter_name if data.exporter_name else None, duration=data.duration.ToTimedelta(), effective_duration=effective_duration, begin_time=begin_time, @@ -411,8 +413,9 @@ async def ListLeases( async def CreateLease( self, *, - selector: str, + selector: str | None, duration: timedelta, + exporter_name: str | None = None, begin_time: datetime | None = None, lease_id: str | None = None, ): @@ -421,7 +424,8 @@ async def CreateLease( lease_pb = client_pb2.Lease( duration=duration_pb, - selector=selector, + selector=selector or "", + exporter_name=exporter_name or "", ) if begin_time: diff --git a/python/packages/jumpstarter/jumpstarter/client/lease.py b/python/packages/jumpstarter/jumpstarter/client/lease.py index 1e431a10b..5b4d54ab4 100644 --- a/python/packages/jumpstarter/jumpstarter/client/lease.py +++ b/python/packages/jumpstarter/jumpstarter/client/lease.py @@ -45,7 +45,8 @@ class Lease(ContextManagerMixin, AsyncContextManagerMixin): channel: Channel duration: timedelta - selector: str + selector: str | None + requested_exporter_name: str | None = None portal: BlockingPortal namespace: str name: str | None = field(default=None) @@ -76,6 +77,7 @@ async def _create(self): self.name = ( await self.svc.CreateLease( selector=self.selector, + exporter_name=self.requested_exporter_name, duration=self.duration, lease_id=self.name, ) diff --git a/python/packages/jumpstarter/jumpstarter/config/client.py b/python/packages/jumpstarter/jumpstarter/config/client.py index 3e9819a3e..1849d01e5 100644 --- a/python/packages/jumpstarter/jumpstarter/config/client.py +++ b/python/packages/jumpstarter/jumpstarter/config/client.py @@ -154,11 +154,14 @@ async def channel(self) -> grpc.aio.Channel: def lease( self, selector: str | None = None, + exporter_name: str | None = None, lease_name: str | None = None, duration: timedelta = timedelta(minutes=30), ): with start_blocking_portal() as portal: - with portal.wrap_async_context_manager(self.lease_async(selector, lease_name, duration, portal)) as lease: + with portal.wrap_async_context_manager( + self.lease_async(selector, exporter_name, lease_name, duration, portal) + ) as lease: yield lease @_blocking_compat @@ -219,14 +222,16 @@ async def list_exporters( @_handle_connection_error async def create_lease( self, - selector: str, duration: timedelta, + selector: str | None = None, + exporter_name: str | None = None, begin_time: datetime | None = None, lease_id: str | None = None, ): svc = ClientService(channel=await self.channel(), namespace=self.metadata.namespace) return await svc.CreateLease( selector=selector, + exporter_name=exporter_name, duration=duration, begin_time=begin_time, lease_id=lease_id, @@ -274,7 +279,8 @@ async def update_lease( @asynccontextmanager async def lease_async( self, - selector: str, + selector: str | None, + exporter_name: str | None, lease_name: str | None, duration: timedelta, portal: BlockingPortal, @@ -298,6 +304,7 @@ async def lease_async( namespace=self.metadata.namespace, name=lease_name, selector=selector, + requested_exporter_name=exporter_name, duration=duration, portal=portal, allow=self.drivers.allow, diff --git a/python/packages/jumpstarter/jumpstarter/config/client_config_test.py b/python/packages/jumpstarter/jumpstarter/config/client_config_test.py index 571a6c72a..64477cc8c 100644 --- a/python/packages/jumpstarter/jumpstarter/config/client_config_test.py +++ b/python/packages/jumpstarter/jumpstarter/config/client_config_test.py @@ -1,7 +1,8 @@ import os import tempfile +from datetime import timedelta from pathlib import Path -from unittest.mock import patch +from unittest.mock import AsyncMock, Mock, patch import pytest import yaml @@ -410,3 +411,35 @@ def test_client_config_delete_does_not_exist_raises(): with pytest.raises(FileNotFoundError): ClientConfigV1Alpha1.delete("xyz") _get_path_mock.assert_called_once_with("xyz") + + +@pytest.mark.asyncio +async def test_create_lease_passes_exporter_name(): + config = ClientConfigV1Alpha1( + alias="testclient", + metadata=ObjectMeta(namespace="default", name="testclient"), + endpoint="jumpstarter.my-lab.com:1443", + token="token", + drivers=ClientConfigV1Alpha1Drivers(allow=["jumpstarter.drivers.*"], unsafe=False), + ) + mock_service = Mock() + mock_service.CreateLease = AsyncMock(return_value="lease") + + with ( + patch("jumpstarter.config.client.ClientConfigV1Alpha1.channel", AsyncMock(return_value=Mock())), + patch("jumpstarter.config.client.ClientService", return_value=mock_service), + ): + result = await config.create_lease( + selector=None, + exporter_name="laptop-test-exporter", + duration=timedelta(minutes=5), + ) + + assert result == "lease" + mock_service.CreateLease.assert_awaited_once_with( + selector=None, + exporter_name="laptop-test-exporter", + duration=timedelta(minutes=5), + begin_time=None, + lease_id=None, + )