Skip to content

fix: fixed the targetCluster cannot be set per event issue in maestro client transportation #44

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

Merged
openshift-merge-bot merged 1 commit into from
Feb 11, 2026
Merged

fix: fixed the targetCluster cannot be set per event issue in maestro client transportation #44

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 4 additions & 5 deletions internal/executor/resource_executor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package executor
import (
"context"
"fmt"
"strings"

"github.com/mitchellh/copystructure"
"github.com/openshift-hyperfleet/hyperfleet-adapter/internal/config_loader"
Expand Down Expand Up @@ -114,7 +113,7 @@ func (re *ResourceExecutor) applyResource(ctx context.Context, resource config_l
} else {
// No Discovery config - lookup by name from manifest
re.log.Debugf(ctx, "Looking up existing resource by name...")
existingResource, err = re.client.GetResource(ctx, gvk, resourceManifest.GetNamespace(), resourceManifest.GetName())
existingResource, err = re.client.GetResource(ctx, gvk, resourceManifest.GetNamespace(), resourceManifest.GetName(), nil)
}

// Fail fast on any error except NotFound (which means resource doesn't exist yet)
Expand Down Expand Up @@ -180,7 +179,7 @@ func (re *ResourceExecutor) applyResource(ctx context.Context, resource config_l

successCtx := logger.WithK8sResult(ctx, "SUCCESS")
re.log.Infof(successCtx, "Resource[%s] processed: operation=%s reason=%s",
resource.Name, strings.ToUpper(string(result.Operation)), result.OperationReason)
resource.Name, result.Operation, result.OperationReason)
Comment on lines 181 to +182
Copy link

@coderabbitai coderabbitai bot Feb 11, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# First, let's find where Operation constants are defined
find . -name "*.go" -type f | head -20

Repository: openshift-hyperfleet/hyperfleet-adapter

Length of output: 837

🏁 Script executed:

# Search for Operation constant definitions in the manifest package
rg -n "Operation\s*=" --type=go -A1 internal/manifest/

Repository: openshift-hyperfleet/hyperfleet-adapter

Length of output: 893

🏁 Script executed:

# Also search for Operation type definitions and constants more broadly
rg -n "const\s+.*Operation" --type=go | head -20

Repository: openshift-hyperfleet/hyperfleet-adapter

Length of output: 65

🏁 Script executed:

# Check the actual values assigned to Operation constants
rg -n "Operation.*=\s*\"" --type=go

Repository: openshift-hyperfleet/hyperfleet-adapter

Length of output: 674

Logging format change confirmed: operation values are now lowercase.

The Operation constants (OperationCreate, OperationUpdate, OperationRecreate, OperationSkip) are defined as lowercase strings ("create", "update", "recreate", "skip" in internal/manifest/generation.go). By removing the strings.ToUpper call, the log output format changes from uppercase (e.g., "CREATE") to lowercase (e.g., "create"). This affects log consistency and any downstream log parsing that may depend on the previous uppercase format.

🤖 Prompt for AI Agents 
In `@internal/executor/resource_executor.go` around lines 182 - 183, The log now
emits operation values in lowercase because the Operation constants
(OperationCreate, OperationUpdate, OperationRecreate, OperationSkip in
internal/manifest/generation.go) are lowercase strings and strings.ToUpper was
removed; restore the previous uppercase log format by uppercasing the operation
when logging (e.g., in resource_executor.go where re.log.Infof is called for
Resource processing) so logs remain consistent with downstream parsers—apply
strings.ToUpper(result.Operation) (or format accordingly) in the log message to
convert the operation to uppercase before logging.

Copy link
Contributor Author

@xueli181114 xueli181114 Feb 11, 2026

Choose a reason for hiding this comment

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

This is my first version. No need to keep consistency

Copy link

@coderabbitai coderabbitai bot Feb 11, 2026

Choose a reason for hiding this comment

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

@xueli181114, understood! Since this is the first version, lowercase operation logging makes sense. Thank you for clarifying.

🧠 Learnings used 
Learnt from: xueli181114
Repo: openshift-hyperfleet/hyperfleet-adapter PR: 39
File: pkg/logger/context.go:35-35
Timestamp: 2026-02-06T06:53:45.757Z
Learning: In the openshift-hyperfleet/hyperfleet-adapter repository, log field keys should be descriptive rather than generic. Use `cluster_id` instead of generic `id` for cluster identifiers in log fields (ClusterIDKey constant and WithClusterID function) to maintain consistency with other resource fields like `resource_id` and improve log searchability and observability.


// Store resource in execution context
if result.Resource != nil {
Expand Down Expand Up @@ -269,7 +268,7 @@ func (re *ResourceExecutor) discoverExistingResource(ctx context.Context, gvk sc
if err != nil {
return nil, fmt.Errorf("failed to render byName template: %w", err)
}
return re.client.GetResource(ctx, gvk, namespace, name)
return re.client.GetResource(ctx, gvk, namespace, name, nil)
}

// Discover by label selector
Expand All @@ -295,7 +294,7 @@ func (re *ResourceExecutor) discoverExistingResource(ctx context.Context, gvk sc
LabelSelector: labelSelector,
}

list, err := re.client.DiscoverResources(ctx, gvk, discoveryConfig)
list, err := re.client.DiscoverResources(ctx, gvk, discoveryConfig, nil)
if err != nil {
return nil, err
}
Expand Down
6 changes: 3 additions & 3 deletions internal/k8s_client/apply.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ func (c *Client) ApplyResource(
gvk := newManifest.GroupVersionKind()
name := newManifest.GetName()

c.log.Infof(ctx, "ApplyResource %s/%s: operation=%s reason=%s",
c.log.Debugf(ctx, "ApplyResource %s/%s: operation=%s reason=%s",
gvk.Kind, name, result.Operation, result.Reason)

// Execute the operation
Expand Down Expand Up @@ -189,7 +189,7 @@ func (c *Client) ApplyResources(
result.Results = append(result.Results, resourceResult)
result.SuccessCount++

c.log.Infof(ctx, "Applied resource %s: operation=%s", r.Name, applyResult.Operation)
c.log.Debugf(ctx, "Applied resource %s: operation=%s reason=%s", r.Name, applyResult.Operation, applyResult.Reason)
}

c.log.Infof(ctx, "Applied %d resources successfully", result.SuccessCount)
Expand Down Expand Up @@ -243,7 +243,7 @@ func (c *Client) waitForDeletion(
c.log.Warnf(ctx, "Context cancelled/timed out while waiting for deletion of %s/%s", gvk.Kind, name)
return fmt.Errorf("context cancelled while waiting for resource deletion: %w", ctx.Err())
case <-ticker.C:
_, err := c.GetResource(ctx, gvk, namespace, name)
_, err := c.GetResource(ctx, gvk, namespace, name, nil)
if err != nil {
// NotFound means the resource is deleted - this is success
if apierrors.IsNotFound(err) {
Expand Down
5 changes: 3 additions & 2 deletions internal/k8s_client/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"encoding/json"

"github.com/openshift-hyperfleet/hyperfleet-adapter/internal/transport_client"
apperrors "github.com/openshift-hyperfleet/hyperfleet-adapter/pkg/errors"
"github.com/openshift-hyperfleet/hyperfleet-adapter/pkg/logger"
apierrors "k8s.io/apimachinery/pkg/api/errors"
Expand Down Expand Up @@ -141,7 +142,7 @@ func (c *Client) CreateResource(ctx context.Context, obj *unstructured.Unstructu
}

// GetResource retrieves a specific Kubernetes resource by GVK, namespace, and name
func (c *Client) GetResource(ctx context.Context, gvk schema.GroupVersionKind, namespace, name string) (*unstructured.Unstructured, error) {
func (c *Client) GetResource(ctx context.Context, gvk schema.GroupVersionKind, namespace, name string, _ transport_client.TransportContext) (*unstructured.Unstructured, error) {
c.log.Infof(ctx, "Getting resource: %s/%s (namespace: %s)", gvk.Kind, name, namespace)

obj := &unstructured.Unstructured{}
Expand Down Expand Up @@ -357,5 +358,5 @@ func (c *Client) PatchResource(ctx context.Context, gvk schema.GroupVersionKind,
c.log.Infof(ctx, "Successfully patched resource: %s/%s", gvk.Kind, name)

// Get the updated resource to return
return c.GetResource(ctx, gvk, namespace, name)
return c.GetResource(ctx, gvk, namespace, name, nil)
}
5 changes: 3 additions & 2 deletions internal/k8s_client/discovery.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"

"github.com/openshift-hyperfleet/hyperfleet-adapter/internal/manifest"
"github.com/openshift-hyperfleet/hyperfleet-adapter/internal/transport_client"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime/schema"
)
Expand All @@ -29,7 +30,7 @@ var BuildLabelSelector = manifest.BuildLabelSelector
// LabelSelector: "app=myapp",
// }
// list, err := client.DiscoverResources(ctx, gvk, discovery)
func (c *Client) DiscoverResources(ctx context.Context, gvk schema.GroupVersionKind, discovery manifest.Discovery) (*unstructured.UnstructuredList, error) {
func (c *Client) DiscoverResources(ctx context.Context, gvk schema.GroupVersionKind, discovery manifest.Discovery, _ transport_client.TransportContext) (*unstructured.UnstructuredList, error) {
list := &unstructured.UnstructuredList{}
list.SetGroupVersionKind(gvk)
if discovery == nil {
Expand All @@ -41,7 +42,7 @@ func (c *Client) DiscoverResources(ctx context.Context, gvk schema.GroupVersionK
c.log.Infof(ctx, "Discovering single resource: %s/%s (namespace: %s)",
gvk.Kind, discovery.GetName(), discovery.GetNamespace())

obj, err := c.GetResource(ctx, gvk, discovery.GetNamespace(), discovery.GetName())
obj, err := c.GetResource(ctx, gvk, discovery.GetNamespace(), discovery.GetName(), nil)
if err != nil {
return list, err
}
Expand Down
5 changes: 3 additions & 2 deletions internal/k8s_client/mock.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"

"github.com/openshift-hyperfleet/hyperfleet-adapter/internal/manifest"
"github.com/openshift-hyperfleet/hyperfleet-adapter/internal/transport_client"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime/schema"
Expand Down Expand Up @@ -40,7 +41,7 @@ func NewMockK8sClient() *MockK8sClient {

// GetResource implements K8sClient.GetResource
// Returns a NotFound error when the resource doesn't exist, matching real K8s client behavior.
func (m *MockK8sClient) GetResource(ctx context.Context, gvk schema.GroupVersionKind, namespace, name string) (*unstructured.Unstructured, error) {
func (m *MockK8sClient) GetResource(ctx context.Context, gvk schema.GroupVersionKind, namespace, name string, _ transport_client.TransportContext) (*unstructured.Unstructured, error) {
// Check explicit error override first
if m.GetResourceError != nil {
return nil, m.GetResourceError
Expand Down Expand Up @@ -149,7 +150,7 @@ func (m *MockK8sClient) ApplyResources(ctx context.Context, resources []Resource
}

// DiscoverResources implements K8sClient.DiscoverResources
func (m *MockK8sClient) DiscoverResources(ctx context.Context, gvk schema.GroupVersionKind, discovery manifest.Discovery) (*unstructured.UnstructuredList, error) {
func (m *MockK8sClient) DiscoverResources(ctx context.Context, gvk schema.GroupVersionKind, discovery manifest.Discovery, _ transport_client.TransportContext) (*unstructured.UnstructuredList, error) {
if m.DiscoverError != nil {
return nil, m.DiscoverError
}
Expand Down
Loading