8000 refactor(actuator): drop XMC_MANAGED_XPROF_GCS_BUCKET env var by CatherineF-dev · Pull Request #840 · google/xmc · GitHub
[go: up one dir, main page]
Skip to content

refactor(actuator): drop XMC_MANAGED_XPROF_GCS_BUCKET env var #840

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
CatherineF-dev merged 4 commits into from
Sep 10, 2026
Merged

refactor(actuator): drop XMC_MANAGED_XPROF_GCS_BUCKET env var #840

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3cf1db7
refactor(actuator): drop XMC_MANAGED_XPROF_GCS_BUCKET env var
CatherineF-dev Sep 9, 2026
fdfc0c1
Merge branch 'main' into refactor/drop-managed-xprof-gcs-bucket-env
CatherineF-dev Sep 9, 2026
5eceeea
test(actuator): update test case name for dropped xprof bucket parameter
CatherineF-dev Sep 10, 2026
9c756f3
Merge branch 'main' into refactor/drop-managed-xprof-gcs-bucket-env
CatherineF-dev Sep 10, 2026
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
29 changes: 12 additions & 17 deletions cmd/actuator/k8s_client/jobset.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -380,27 +380,24 @@ func (c *Client) k8sJobToReplicatedJob(ctx context.Context, job *esspb.Kubernete
}
}

// injectManagedXprofGCSEnvVars appends the managed-xprof GCS destination env vars to
// every container of the replicated job so all workloads in the pod know where to
// write xprof output, not just the primary container. No-op when no bucket was
// resolved.
func injectManagedXprofGCSEnvVars(rj *jobsetv1alpha2.ReplicatedJob, xprofBucketURL, xprofObjectPath string) {
if xprofBucketURL == "" && xprofObjectPath == "" {
// injectManagedXprofGCSEnvVars appends the per-work-unit managed-xprof GCS object
// folder env var to every container of the replicated job so all workloads in the
// pod know where to write xprof output, not just the primary container. No-op when
// no destination was resolved.
func injectManagedXprofGCSEnvVars(rj *jobsetv1alpha2.ReplicatedJob, xprofObjectPath string) {
if xprofObjectPath == "" {
return
}
injectManagedXprofGCSEnvVarsToContainers(rj.Template.Spec.Template.Spec.Containers, xprofBucketURL, xprofObjectPath)
injectManagedXprofGCSEnvVarsToContainers(rj.Template.Spec.Template.Spec.InitContainers, xprofBucketURL, xprofObjectPath)
injectManagedXprofGCSEnvVarsToContainers(rj.Template.Spec.Template.Spec.Containers, xprofObjectPath)
injectManagedXprofGCSEnvVarsToContainers(rj.Template.Spec.Template.Spec.InitContainers, xprofObjectPath)
}

// injectManagedXprofGCSEnvVarsToContainers appends the managed-xprof GCS destination
// env vars to each container in the provided slice in place.
func injectManagedXprofGCSEnvVarsToContainers(containers []corev1.Container, xprofBucketURL, xprofObjectPath string) {
// injectManagedXprofGCSEnvVarsToContainers appends the per-work-unit managed-xprof
// GCS object folder env var to each container in the provided slice in place.
func injectManagedXprofGCSEnvVarsToContainers(containers []corev1.Container, xprofObjectPath string) {
for i := range containers {
// Index into the slice so appends write back to the JobSet in place.
container := &containers[i]
if xprofBucketURL != "" {
container.Env = append(container.Env, corev1.EnvVar{Name: constants.XmcEnvVarManagedXprofGCSBucket, Value: xprofBucketURL})
}
if xprofObjectPath != "" {
container.Env = append(container.Env, corev1.EnvVar{Name: constants.XmcEnvVarManagedXprofGCSObjectFolder, Value: xprofObjectPath})
}
Comment thread
CatherineF-dev marked this conversation as resolved.
Resolved
Expand Down Expand Up @@ -628,20 +625,18 @@ func (c *Client) constructJobSet(ctx context.Context, workUnit *esspb.WorkUnit,
// managed-xprof GCS bucket. This is intentionally decoupled from the ManagedXprof
// artifact: the workload only needs to know where to write xprof output,
// regardless of whether or when the artifact is attached.
var xprofBucketURL string
var xprofObjectPath string
if bucket, bucketErr := c.ManagedXProfGCSBucket(ctx); bucketErr != nil {
logging.Warn(ctx, "Failed to resolve managed-xprof GCS path via Actuator", "error", bucketErr)
} else if bucket != "" {
xprofBucketURL = bucket
xprofObjectPath = fmt.Sprintf("%s/xmc-%s-%s-%s", strings.TrimSuffix(bucket, "/"), xid, wid, strconv.FormatInt(operationTime.Unix(), 10))
}

var replicatedJobs []jobsetv1alpha2.ReplicatedJob
if workUnit.GetExecutable().GetJobset() != nil {
for _, job := range workUnit.GetExecutable().GetJobset().GetJobs() {
rj := c.k8sJobToReplicatedJob(ctx, job, jobLabels, defaultAnnotations)
injectManagedXprofGCSEnvVars(&rj, xprofBucketURL, xprofObjectPath)
injectManagedXprofGCSEnvVars(&rj, xprofObjectPath)
replicatedJobs = append(replicatedJobs, rj)
}
}
Expand Down
20 changes: 3 additions & 17 deletions cmd/actuator/k8s_client/jobset_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2860,13 +2860,11 @@ func TestConstructJobSet_ManagedXprofGCSInjection(t *testing.T) {
tests := []struct {
name string
gcsPathEnv string
wantGCSBucket string
wantGCSObject string
}{
{
name: "gcs path env present injects GCS env",
gcsPathEnv: "gs://my-awesome-profile-bucket",
wantGCSBucket: "gs://my-awesome-profile-bucket",
wantGCSObject: "gs://my-awesome-profile-bucket/xmc-123-456-1764149578",
},
{
Expand Down Expand Up @@ -2925,9 +2923,6 @@ func TestConstructJobSet_ManagedXprofGCSInjection(t *testing.T) {
envMap[ev.Name] = ev.Value
}

if got := envMap[constants.XmcEnvVarManagedXprofGCSBucket]; got != tc.wantGCSBucket {
t.Errorf("GCS bucket env = %q, want %q", got, tc.wantGCSBucket)
}
if got := envMap[constants.XmcEnvVarManagedXprofGCSObjectFolder]; got != tc.wantGCSObject {
t.Errorf("GCS object folder env = %q, want %q", got, tc.wantGCSObject)
}
Expand All @@ -2945,7 +2940,6 @@ func TestConstructJobSet_ManagedXprofGCSInjection(t *testing.T) {

func TestInjectManagedXprofGCSEnvVars_AllContainers(t *testing.T) {
const (
wantBucket = "gs://my-bucket"
wantObject = "gs://my-bucket/xmc-1-1-100"
)

Expand All @@ -2967,33 +2961,29 @@ func TestInjectManagedXprofGCSEnvVars_AllContainers(t *testing.T) {
tests := []struct {
name string
numContainers int
bucket string
object string
wantInjected bool
}{
{
name: "injects into every container",
numContainers: 3,
bucket: wantBucket,
object: wantObject,
wantInjected: true,
},
{
name: "single container still injected",
numContainers: 1,
bucket: wantBucket,
object: wantObject,
wantInjected: true,
},
{
name: "no bucket or object is a no-op",
name: "empty object is a no-op",
numContainers: 2,
wantInjected: false,
},
{
name: "no containers is a safe no-op",
numContainers: 0,
bucket: wantBucket,
object: wantObject,
wantInjected: false,
},
Expand All @@ -3002,7 +2992,7 @@ func TestInjectManagedXprofGCSEnvVars_AllContainers(t *testing.T) {
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
rj := newReplicatedJob(tc.numContainers)
injectManagedXprofGCSEnvVars(rj, tc.bucket, tc.object)
injectManagedXprofGCSEnvVars(rj, tc.object)

for _, containers := range [][]corev1.Container{
rj.Template.Spec.Template.Spec.Containers,
Expand All @@ -3013,16 +3003,12 @@ func TestInjectManagedXprofGCSEnvVars_AllContainers(t *testing.T) {
for _, ev := range containers[i].Env {
envMap[ev.Name] = ev.Value
}
gotBucket, hasBucket := envMap[constants.XmcEnvVarManagedXprofGCSBucket]
gotObject, hasObject := envMap[constants.XmcEnvVarManagedXprofGCSObjectFolder]
if tc.wantInjected {
if gotBucket != tc.bucket {
t.Errorf("container[%d] %s = %q, want %q", i, constants.XmcEnvVarManagedXprofGCSBucket, gotBucket, tc.bucket)
}
if gotObject != tc.object {
t.Errorf("container[%d] %s = %q, want %q", i, constants.XmcEnvVarManagedXprofGCSObjectFolder, gotObject, tc.object)
}
} else if hasBucket || hasObject {
} else if hasObject {
t.Errorf("container[%d] unexpectedly got managed-xprof env vars: %v", i, envMap)
}
}
Expand Down
3 changes: 0 additions & 3 deletions internal/kubernetes/common/constants/constants.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,6 @@ const (
// xprof is not configured for the cluster.
XmcEnvVarManagedXprofGCSPath = "MANAGED_XPROF_GCS_PATH"

// XmcEnvVarManagedXprofGCSBucket is the environment variable containing the GCS bucket for managed profiling, which is shared by the whole cluster.
XmcEnvVarManagedXprofGCSBucket = "XMC_MANAGED_XPROF_GCS_BUCKET"

// XmcEnvVarManagedXprofGCSObjectFolder is the environment variable containing the GCS object folder for managed profiling, which is per workunit.
XmcEnvVarManagedXprofGCSObjectFolder = "XMC_MANAGED_XPROF_GCS_OBJECT_FOLDER"

Expand Down
Loading
0