From c3fce25711215cd8b0f635ec97135a98658fe7f1 Mon Sep 17 00:00:00 2001 From: Diamon Wiggins Date: Thu, 2 Jan 2025 18:39:48 -0500 Subject: [PATCH 1/2] add preexecute property to runpod --- .../troubleshoot/v1beta2/collector_shared.go | 6 + pkg/collect/run_pod.go | 138 +++++++++++++++++- pkg/k8sutil/config.go | 5 + 3 files changed, 148 insertions(+), 1 deletion(-) diff --git a/pkg/apis/troubleshoot/v1beta2/collector_shared.go b/pkg/apis/troubleshoot/v1beta2/collector_shared.go index 554f732e4..9664465d0 100644 --- a/pkg/apis/troubleshoot/v1beta2/collector_shared.go +++ b/pkg/apis/troubleshoot/v1beta2/collector_shared.go @@ -9,6 +9,7 @@ import ( authorizationv1 "k8s.io/api/authorization/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" ) type CollectorMeta struct { @@ -112,6 +113,11 @@ type RunPod struct { ImagePullSecret *ImagePullSecrets `json:"imagePullSecret,omitempty" yaml:"imagePullSecret,omitempty"` PodSpec corev1.PodSpec `json:"podSpec,omitempty" yaml:"podSpec,omitempty"` Annotations map[string]string `json:"annotations,omitempty" yaml:"annotations,omitempty"` + PreExecute []PreExecuteSpec `json:"preExecute,omitempty" yaml:"preExecute,omitempty"` +} + +type PreExecuteSpec struct { + Resource runtime.RawExtension `json:"resource" yaml:"resource"` } type RunDaemonSet struct { diff --git a/pkg/collect/run_pod.go b/pkg/collect/run_pod.go index e430d6c99..3d8a504aa 100644 --- a/pkg/collect/run_pod.go +++ b/pkg/collect/run_pod.go @@ -23,7 +23,13 @@ import ( kuberneteserrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/util/wait" + "k8s.io/client-go/discovery" + "k8s.io/client-go/discovery/cached/memory" + "k8s.io/client-go/dynamic" + "k8s.io/client-go/restmapper" ) type CollectRunPod struct { @@ -53,6 +59,37 @@ func (c *CollectRunPod) Collect(progressChan chan<- interface{}) (result Collect return nil, errors.Wrap(err, "failed to create client from config") } + // Create dynamic client for pre-execute resources + dynamicClient, err := dynamic.NewForConfig(c.ClientConfig) + if err != nil { + return nil, errors.Wrap(err, "failed to create dynamic client") + } + + // Track created resources for cleanup + var createdResources []resourceRef + + // Execute pre-execute resources + if len(c.Collector.PreExecute) > 0 { + for _, preExec := range c.Collector.PreExecute { + ref, err := executePreResource(ctx, dynamicClient, c.ClientConfig, c.Collector.Namespace, &preExec) + if err != nil { + // Clean up any resources that were created before the error + cleanupPreExecuteResources(ctx, dynamicClient, createdResources) + return nil, errors.Wrap(err, "failed to execute pre-execute resource") + } + createdResources = append(createdResources, ref) + } + } + + // Ensure cleanup happens after pod collection + defer func() { + if len(createdResources) > 0 { + if cleanupErr := cleanupPreExecuteResources(ctx, dynamicClient, createdResources); cleanupErr != nil { + klog.Errorf("Failed to cleanup pre-execute resources: %v", cleanupErr) + } + } + }() + pod, err := runPodWithSpec(ctx, client, c.Collector) if err != nil { return nil, errors.Wrap(err, "failed to run pod") @@ -175,7 +212,6 @@ func runWithoutTimeout(ctx context.Context, bundlePath string, clientConfig *res } } } - time.Sleep(time.Second * 1) } @@ -459,6 +495,106 @@ func deletePod(ctx context.Context, client *kubernetes.Clientset, pod *corev1.Po } } +type resourceRef struct { + GroupVersionResource schema.GroupVersionResource + Namespace string + Name string +} + +func executePreResource(ctx context.Context, client dynamic.Interface, clientConfig *rest.Config, namespace string, preExec *troubleshootv1beta2.PreExecuteSpec) (resourceRef, error) { + // Convert RawExtension to Unstructured + var obj unstructured.Unstructured + if err := json.Unmarshal(preExec.Resource.Raw, &obj); err != nil { + return resourceRef{}, errors.Wrap(err, "failed to unmarshal pre-execute resource") + } + + // Ensure namespace is set before any operations + if obj.GetNamespace() == "" { + if namespace == "" { + namespace = "default" // Fallback to default namespace if none provided + } + obj.SetNamespace(namespace) + } + + klog.V(2).Infof("Attempting to create resource: Kind=%s, APIVersion=%s, Name=%s, Namespace=%s", + obj.GetKind(), + obj.GetAPIVersion(), + obj.GetName(), + obj.GetNamespace()) + + // Create discovery client and REST mapper + dc, err := discovery.NewDiscoveryClientForConfig(clientConfig) + if err != nil { + return resourceRef{}, errors.Wrap(err, "failed to create discovery client") + } + mapper := restmapper.NewDeferredDiscoveryRESTMapper(memory.NewMemCacheClient(dc)) + + // Get the GVR for the resource + gvk := obj.GroupVersionKind() + mapping, err := mapper.RESTMapping(gvk.GroupKind(), gvk.Version) + if err != nil { + return resourceRef{}, errors.Wrap(err, "failed to get REST mapping") + } + + // Log the mapping details + klog.V(2).Infof("Resource mapping: Group=%s, Version=%s, Resource=%s", + mapping.Resource.Group, + mapping.Resource.Version, + mapping.Resource.Resource) + + // Create the resource + if obj.GetNamespace() == "" { + obj.SetNamespace(namespace) + } + + created, err := client.Resource(mapping.Resource).Namespace(obj.GetNamespace()).Create(ctx, &obj, metav1.CreateOptions{}) + if err != nil { + // Log more details about the error + klog.Errorf("Failed to create resource: %v", err) + return resourceRef{}, errors.Wrap(err, "failed to create pre-execute resource") + } + + ref := resourceRef{ + GroupVersionResource: mapping.Resource, + Namespace: created.GetNamespace(), + Name: created.GetName(), + } + + // Try listing resources first to check permissions + _, err = client.Resource(mapping.Resource).Namespace(obj.GetNamespace()).List(ctx, metav1.ListOptions{}) + if err != nil { + klog.Errorf("Permission check failed - cannot list resources: %v", err) + } + + // After getting the discovery client, list available resources + resources, err := dc.ServerResourcesForGroupVersion(gvk.GroupVersion().String()) + if err != nil { + klog.Errorf("Failed to get resources for group version %s: %v", gvk.GroupVersion().String(), err) + } else { + klog.V(2).Infof("Available resources for %s:", gvk.GroupVersion().String()) + for _, r := range resources.APIResources { + klog.V(2).Infof(" - %s (namespaced: %v)", r.Name, r.Namespaced) + } + } + + return ref, nil +} + +func cleanupPreExecuteResources(ctx context.Context, client dynamic.Interface, resources []resourceRef) error { + var errs []error + for _, res := range resources { + err := client.Resource(res.GroupVersionResource).Namespace(res.Namespace).Delete(ctx, res.Name, metav1.DeleteOptions{}) + if err != nil && !kuberneteserrors.IsNotFound(err) { + errs = append(errs, errors.Wrapf(err, "failed to delete resource %s/%s", res.Namespace, res.Name)) + } + } + + if len(errs) > 0 { + return errors.Errorf("failed to cleanup resources: %v", errs) + } + return nil +} + func createPodStruct(runPodCollector *troubleshootv1beta2.RunPod) corev1.Pod { podLabels := make(map[string]string) podLabels["troubleshoot-role"] = "run-collector" diff --git a/pkg/k8sutil/config.go b/pkg/k8sutil/config.go index 82d30545b..b6db5c16f 100644 --- a/pkg/k8sutil/config.go +++ b/pkg/k8sutil/config.go @@ -5,6 +5,7 @@ import ( "k8s.io/cli-runtime/pkg/genericclioptions" "k8s.io/client-go/rest" "k8s.io/client-go/tools/clientcmd" + "k8s.io/apimachinery/pkg/api/meta" ) var ( @@ -26,3 +27,7 @@ func GetKubeconfig() clientcmd.ClientConfig { func GetRESTConfig() (*rest.Config, error) { return kubernetesConfigFlags.ToRESTConfig() } + +func GetRESTMapper() (meta.RESTMapper, error) { + return kubernetesConfigFlags.ToRESTMapper() +} From 07a414d49eb8c81ed34092286a017fe3bac305c3 Mon Sep 17 00:00:00 2001 From: Diamon Wiggins Date: Thu, 2 Jan 2025 20:04:51 -0500 Subject: [PATCH 2/2] add preexecute property to runpod --- .../troubleshoot/v1beta2/collector_shared.go | 18 ++-- pkg/collect/run_pod.go | 94 +++++++++---------- 2 files changed, 51 insertions(+), 61 deletions(-) diff --git a/pkg/apis/troubleshoot/v1beta2/collector_shared.go b/pkg/apis/troubleshoot/v1beta2/collector_shared.go index 9664465d0..6c0fac18a 100644 --- a/pkg/apis/troubleshoot/v1beta2/collector_shared.go +++ b/pkg/apis/troubleshoot/v1beta2/collector_shared.go @@ -107,17 +107,13 @@ type Run struct { type RunPod struct { CollectorMeta `json:",inline" yaml:",inline"` - Name string `json:"name,omitempty" yaml:"name,omitempty"` - Namespace string `json:"namespace" yaml:"namespace"` - Timeout string `json:"timeout,omitempty" yaml:"timeout,omitempty"` - ImagePullSecret *ImagePullSecrets `json:"imagePullSecret,omitempty" yaml:"imagePullSecret,omitempty"` - PodSpec corev1.PodSpec `json:"podSpec,omitempty" yaml:"podSpec,omitempty"` - Annotations map[string]string `json:"annotations,omitempty" yaml:"annotations,omitempty"` - PreExecute []PreExecuteSpec `json:"preExecute,omitempty" yaml:"preExecute,omitempty"` -} - -type PreExecuteSpec struct { - Resource runtime.RawExtension `json:"resource" yaml:"resource"` + Name string `json:"name,omitempty" yaml:"name,omitempty"` + Namespace string `json:"namespace" yaml:"namespace"` + Timeout string `json:"timeout,omitempty" yaml:"timeout,omitempty"` + ImagePullSecret *ImagePullSecrets `json:"imagePullSecret,omitempty" yaml:"imagePullSecret,omitempty"` + PodSpec corev1.PodSpec `json:"podSpec,omitempty" yaml:"podSpec,omitempty"` + Annotations map[string]string `json:"annotations,omitempty" yaml:"annotations,omitempty"` + PreExecute []runtime.RawExtension `json:"preExecute,omitempty"` } type RunDaemonSet struct { diff --git a/pkg/collect/run_pod.go b/pkg/collect/run_pod.go index 3d8a504aa..88a282301 100644 --- a/pkg/collect/run_pod.go +++ b/pkg/collect/run_pod.go @@ -22,8 +22,10 @@ import ( "k8s.io/klog/v2" kuberneteserrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/client-go/discovery" @@ -70,8 +72,8 @@ func (c *CollectRunPod) Collect(progressChan chan<- interface{}) (result Collect // Execute pre-execute resources if len(c.Collector.PreExecute) > 0 { - for _, preExec := range c.Collector.PreExecute { - ref, err := executePreResource(ctx, dynamicClient, c.ClientConfig, c.Collector.Namespace, &preExec) + for _, rawResource := range c.Collector.PreExecute { + ref, err := executePreResource(ctx, dynamicClient, c.ClientConfig, c.Collector.Namespace, &rawResource) if err != nil { // Clean up any resources that were created before the error cleanupPreExecuteResources(ctx, dynamicClient, createdResources) @@ -501,83 +503,75 @@ type resourceRef struct { Name string } -func executePreResource(ctx context.Context, client dynamic.Interface, clientConfig *rest.Config, namespace string, preExec *troubleshootv1beta2.PreExecuteSpec) (resourceRef, error) { - // Convert RawExtension to Unstructured - var obj unstructured.Unstructured - if err := json.Unmarshal(preExec.Resource.Raw, &obj); err != nil { - return resourceRef{}, errors.Wrap(err, "failed to unmarshal pre-execute resource") +func isNamespacedResource(dc discovery.DiscoveryInterface, mapping *meta.RESTMapping) (bool, error) { + resources, err := dc.ServerResourcesForGroupVersion(mapping.GroupVersionKind.GroupVersion().String()) + if err != nil { + return false, errors.Wrap(err, "failed to get resources for group version") } - // Ensure namespace is set before any operations - if obj.GetNamespace() == "" { - if namespace == "" { - namespace = "default" // Fallback to default namespace if none provided + for _, r := range resources.APIResources { + if r.Name == mapping.Resource.Resource { + return r.Namespaced, nil } - obj.SetNamespace(namespace) } - klog.V(2).Infof("Attempting to create resource: Kind=%s, APIVersion=%s, Name=%s, Namespace=%s", - obj.GetKind(), - obj.GetAPIVersion(), - obj.GetName(), - obj.GetNamespace()) + return false, errors.New("resource type not found in api resources") +} + +func executePreResource(ctx context.Context, client dynamic.Interface, clientConfig *rest.Config, namespace string, rawResource *runtime.RawExtension) (resourceRef, error) { + var obj unstructured.Unstructured + if err := json.Unmarshal(rawResource.Raw, &obj); err != nil { + return resourceRef{}, errors.Wrap(err, "failed to unmarshal pre-execute resource") + } - // Create discovery client and REST mapper dc, err := discovery.NewDiscoveryClientForConfig(clientConfig) if err != nil { return resourceRef{}, errors.Wrap(err, "failed to create discovery client") } mapper := restmapper.NewDeferredDiscoveryRESTMapper(memory.NewMemCacheClient(dc)) - // Get the GVR for the resource gvk := obj.GroupVersionKind() mapping, err := mapper.RESTMapping(gvk.GroupKind(), gvk.Version) if err != nil { return resourceRef{}, errors.Wrap(err, "failed to get REST mapping") } - // Log the mapping details - klog.V(2).Infof("Resource mapping: Group=%s, Version=%s, Resource=%s", - mapping.Resource.Group, - mapping.Resource.Version, - mapping.Resource.Resource) - - // Create the resource - if obj.GetNamespace() == "" { - obj.SetNamespace(namespace) - } - - created, err := client.Resource(mapping.Resource).Namespace(obj.GetNamespace()).Create(ctx, &obj, metav1.CreateOptions{}) + isNamespaced, err := isNamespacedResource(dc, mapping) if err != nil { - // Log more details about the error - klog.Errorf("Failed to create resource: %v", err) - return resourceRef{}, errors.Wrap(err, "failed to create pre-execute resource") + return resourceRef{}, errors.Wrap(err, "failed to check if resource is namespaced") } - ref := resourceRef{ - GroupVersionResource: mapping.Resource, - Namespace: created.GetNamespace(), - Name: created.GetName(), + if isNamespaced { + if obj.GetNamespace() == "" { + if namespace == "" { + namespace = "default" + } + obj.SetNamespace(namespace) + } + } else { + obj.SetNamespace("") } - // Try listing resources first to check permissions - _, err = client.Resource(mapping.Resource).Namespace(obj.GetNamespace()).List(ctx, metav1.ListOptions{}) - if err != nil { - klog.Errorf("Permission check failed - cannot list resources: %v", err) + var created *unstructured.Unstructured + if isNamespaced { + created, err = client.Resource(mapping.Resource).Namespace(obj.GetNamespace()).Create(ctx, &obj, metav1.CreateOptions{}) + } else { + created, err = client.Resource(mapping.Resource).Create(ctx, &obj, metav1.CreateOptions{}) } - // After getting the discovery client, list available resources - resources, err := dc.ServerResourcesForGroupVersion(gvk.GroupVersion().String()) if err != nil { - klog.Errorf("Failed to get resources for group version %s: %v", gvk.GroupVersion().String(), err) - } else { - klog.V(2).Infof("Available resources for %s:", gvk.GroupVersion().String()) - for _, r := range resources.APIResources { - klog.V(2).Infof(" - %s (namespaced: %v)", r.Name, r.Namespaced) + msg := "failed to create pre-execute resource" + if kuberneteserrors.IsAlreadyExists(err) { + msg = fmt.Sprintf("resource %s/%s of type %s already exists", obj.GetNamespace(), obj.GetName(), mapping.Resource.Resource) } + return resourceRef{}, errors.Wrap(err, msg) } - return ref, nil + return resourceRef{ + GroupVersionResource: mapping.Resource, + Namespace: created.GetNamespace(), + Name: created.GetName(), + }, nil } func cleanupPreExecuteResources(ctx context.Context, client dynamic.Interface, resources []resourceRef) error {