Skip to content

Conversation

@xiangchunfu
Copy link
Contributor

List all the issues related to this PR

  • New Feature
  • Enhancement
  • Bug fix
  • Tests
  • Documentation
  • CI/CD

What environments does this code impact?

  • Automation (CI, tools, etc)
  • Cloud
  • Operator Managed Deployments
  • None

How was this code tested?

  • assisted-test-infra environment
  • dev-scripts environment
  • Reviewer's test appreciated
  • Waiting for CI to do a full test run
  • Manual (Elaborate on how it was tested)
  • No tests needed

Checklist

  • Title and description added to both, commit and PR.
  • Relevant issues have been associated (see CONTRIBUTING guide)
  • This change does not require a documentation update (docstring, docs, README, etc)
  • Does this change include unit-tests (note that code changes require unit-tests)

Reviewers Checklist

  • Are the title and description (in both PR and commit) meaningful and clear?
  • Is there a bug required (and linked) for this change?
  • Should this PR be backported?

@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 22, 2025
@openshift-ci
Copy link

openshift-ci bot commented Oct 22, 2025

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@openshift-ci openshift-ci bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Oct 22, 2025
@openshift-ci
Copy link

openshift-ci bot commented Oct 22, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: xiangchunfu
Once this PR has been reviewed and has the lgtm label, please assign javipolo for approval. For more information see the Code Review Process.

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

Needs approval from an approver in each of these files:

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

@xiangchunfu
Copy link
Contributor Author

Fail to install osc operator and kata runtimeclass in Assisted installer UI. This is part of bootkube.logs:
....
Oct 22 01:35:28 dell-per7525-13.lab.eng.pek2.redhat.com cluster-bootstrap[22397]: [#127] failed to create some manifests:
Oct 22 01:35:28 dell-per7525-13.lab.eng.pek2.redhat.com cluster-bootstrap[22397]: "50_openshift-osc_kataconfig.yaml": unable to get REST mapping for "50_openshift-osc_kataconfig.yaml": no matches for kind "KataConfig" in version "kataconfiguration.openshift.io/v1"
Oct 22 01:35:28 dell-per7525-13.lab.eng.pek2.redhat.com bootkube.sh[22359]: [#127] failed to create some manifests:
Oct 22 01:35:28 dell-per7525-13.lab.eng.pek2.redhat.com bootkube.sh[22359]: "50_openshift-osc_kataconfig.yaml": unable to get REST mapping for "50_openshift-osc_kataconfig.yaml": no matches for kind "KataConfig" in version "kataconfiguration.openshift.io/v1"
Oct 22 01:35:28 dell-per7525-13.lab.eng.pek2.redhat.com cluster-bootstrap[22397]: [#128] failed to create some manifests:
Oct 22 01:35:28 dell-per7525-13.lab.eng.pek2.redhat.com cluster-bootstrap[22397]: "50_openshift-osc_kataconfig.yaml": unable to get REST mapping for "50_openshift-osc_kataconfig.yaml": no matches for kind "KataConfig" in version "kataconfiguration.openshift.io/v1"
Oct 22 01:35:28 dell-per7525-13.lab.eng.pek2.redhat.com bootkube.sh[22359]: [#128] failed to create some manifests:
Oct 22 01:35:28 dell-per7525-13.lab.eng.pek2.redhat.com bootkube.sh[22359]: "50_openshift-osc_kataconfig.yaml": unable to get REST mapping for "50_openshift-osc_kataconfig.yaml": no matches for kind "KataConfig" in version "kataconfiguration.openshift.io/v1"
......

Comment on lines +116 to +123
const oscKataConfigManifest = `
apiVersion: kataconfiguration.openshift.io/v1
kind: KataConfig
metadata:
name: cluster-kataconfig
spec:
enablePeerPods: false
logLevel: info
`
Copy link
Contributor

Choose a reason for hiding this comment

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

@jhernand it looks like this operator needs this CRD first. I don't see any operator adding CRDs, would you recommend this path or do you know any other better way to achieve this?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this may not work. These manifests are created all together, so there is no guarantee that the operator (created as a result of the subscription) will be already available, and therefore the custom resource definitions will not be available yet. It would be better to create it as part of the "custom manifests". Those are the second return value of the Manifests function (currently nil):

return openshiftManifests, nil, err

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Following this suggestion. I updated this function and it works well now.

@xiangchunfu
Copy link
Contributor Author

Test result:

oc get operators

NAME AGE
sandboxed-containers-operator.openshift-sandboxed-containers-op 108m

oc get runtimeclass

NAME HANDLER AGE
kata kata 76m

#cat kata-pod.yaml
apiVersion: v1
kind: Pod
metadata:
name: kata-pod
namespace: default
spec:
runtimeClassName: kata
containers:

  • image: quay.io/prometheus/busybox
    name: busybox
    resources: {}
    command: ["sleep", "infinity"]
    dns_search: []
    securityContext:
    privileged: true

oc apply -f kata-pod.yaml

oc get pods

NAME READY STATUS RESTARTS AGE
kata-pod 1/1 Running 0 101s

@xiangchunfu xiangchunfu marked this pull request as ready for review October 28, 2025 06:13
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 28, 2025
@openshift-ci openshift-ci bot requested review from carbonin and eliorerz October 28, 2025 06:14
@xiangchunfu
Copy link
Contributor Author

When we choose to install "OpenShift sandboxed containers" during the installation process. Openshift sandboxed containers operator and kata runtimeclass will be installed at the same time.
Screenshot at 2025-10-28 14-57-23

@codecov
Copy link

codecov bot commented Oct 28, 2025

Codecov Report

❌ Patch coverage is 75.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 43.22%. Comparing base (e994ff5) to head (6983d1e).
⚠️ Report is 29 commits behind head on master.

Files with missing lines Patch % Lines
internal/operators/osc/manifest.go 75.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #8140      +/-   ##
==========================================
+ Coverage   43.12%   43.22%   +0.09%     
==========================================
  Files         404      404              
  Lines       69775    69942     +167     
==========================================
+ Hits        30093    30231     +138     
- Misses      36989    37004      +15     
- Partials     2693     2707      +14     
Files with missing lines Coverage Δ
internal/operators/osc/manifest.go 73.91% <75.00%> (-0.45%) ⬇️

... and 8 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@xiangchunfu
Copy link
Contributor Author

/retest

@xiangchunfu
Copy link
Contributor Author

/test edge-e2e-ai-operator-ztp

@xiangchunfu
Copy link
Contributor Author

/test edge-lint

1 similar comment
@xiangchunfu
Copy link
Contributor Author

/test edge-lint

@openshift-ci
Copy link

openshift-ci bot commented Oct 30, 2025

@xiangchunfu: 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/edge-lint 6983d1e link true /test edge-lint

Full PR test history. Your PR dashboard.

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.

@xiangchunfu
Copy link
Contributor Author

xiangchunfu commented Oct 30, 2025

@rccrdpccl @jhernand Could you help to continue reviewing it? Thanks!

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

Labels

size/S Denotes a PR that changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants