Skip to content
This repository was archived by the owner on Sep 9, 2026. It is now read-only.

Map explicit ComputeInstance fields to Kubernetes CR - #309

Merged
akshaynadkarni merged 4 commits into
osac-project:mainfrom
tzvatot:feature/ci-explicit-fields-mgmt-23103
Mar 3, 2026
Merged

akshaynadkarni merged 4 commits into
osac-project:mainfrom
tzvatot:feature/ci-explicit-fields-mgmt-23103

Conversation

@tzvatot

@tzvatot tzvatot commented Feb 25, 2026

Copy link
Copy Markdown
Contributor

Summary

The fulfillment-controller's buildSpec() was only mapping templateID, templateParameters, and restartRequestedAt to the Kubernetes CR. The new CRD requires explicit spec fields (cores, memoryGiB, image, etc.) — without this change, CRD validation rejects the CR.

Changes

  • Added addExplicitFields() to map all new spec fields from the fulfillment API to the K8s CR:
    cores, memoryGiB, image, bootDisk, additionalDisks, runStrategy, sshKey, userDataSecretRef
  • Fixed int32 panic: Kubernetes unstructured.SetNestedField doesn't support int32 — cast to int64
  • Fixed []map[string]any[]any for additionalDisks slice (required by unstructured)
  • Added unit tests for both presence and absence of explicit fields

E2E Validated

Full stack test passed on vmaas-dev: CLI → fulfillment-service → operator → provisioning → Running state.

Related

The fulfillment-controller now passes explicit ComputeInstance fields
(cores, memoryGiB, image, bootDisk, additionalDisks, runStrategy,
sshKey, userDataSecretRef) from the fulfillment API through to the
Kubernetes CR spec. Without this, the CRD validation rejects CRs
missing the required explicit fields.

Adds tests for both presence and absence of explicit fields.

https://issues.redhat.com/browse/MGMT-23103

Generated with [Claude Code](https://claude.com/claude-code)
Kubernetes unstructured.SetNestedField panics on int32 values.
Cast cores, memoryGiB, and disk sizeGiB to int64 for compatibility.
Also use []any for additionalDisks slice (not []map[string]any).

https://issues.redhat.com/browse/MGMT-23103

Generated with [Claude Code](https://claude.com/claude-code)
}
if ciSpec.HasUserDataSecretRef() {
spec["userDataSecretRef"] = map[string]any{
"name": ciSpec.GetUserDataSecretRef(),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

the secret must be created from the string, no? osac-project/enhancement-proposals#21 (comment)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

You're right. The fulfillment-service needs to create the Secret from the user data content, as discussed in the EP. I'll update this PR to create the Secret in the tenant namespace and pass the reference to the CR.

Instead of passing userDataSecretRef as a plain name reference to the CR,
the fulfillment-service now creates a Secret in the hub namespace from
the user data content provided via the API.

- Add ensureUserDataSecret with create/update sub-functions
- Add deleteUserDataSecret for cleanup on CI deletion
- Add gvks.Secret constant and userDataSecretSuffix/userDataSecretKey constants
- Separate Secret lifecycle from spec building (addExplicitFields stays pure)
- Add userDataSecretName field to task struct

Generated with [Claude Code](https://claude.com/claude-code)
@tzvatot
tzvatot force-pushed the feature/ci-explicit-fields-mgmt-23103 branch 2 times, most recently from dc780fd to 4a5a84d Compare February 26, 2026 13:03
Instead of passing userDataSecretRef as a plain name reference to the CR,
the fulfillment-service creates a Secret in the hub namespace from the
user data content provided via the API.

- Add ensureUserDataSecret that creates Secret with owner reference
- Owner reference enables automatic cleanup via K8s garbage collector
- Secret is immutable: create once, skip if AlreadyExists
- CR is created first, then Secret (standard K8s owner reference pattern)
- Add gvks.Secret constant and userDataSecretSuffix/userDataSecretKey constants
- Separate Secret lifecycle from spec building (addExplicitFields stays pure)

Generated with [Claude Code](https://claude.com/claude-code)

@akshaynadkarni akshaynadkarni left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@openshift-ci

openshift-ci Bot commented Mar 3, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: akshaynadkarni, tzvatot

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:
  • OWNERS [akshaynadkarni,tzvatot]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@akshaynadkarni
akshaynadkarni merged commit b16797c into osac-project:main Mar 3, 2026
19 of 21 checks passed
@openshift-ci

openshift-ci Bot commented Mar 3, 2026

Copy link
Copy Markdown

@tzvatot: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/unit b982dc8 link unknown /test unit

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants