Skip to content

KATA-3493: toml file should include image annotations #497

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.

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
merged 1 commit into from
Jan 17, 2025

Conversation

balintTobik
Copy link
Contributor

@balintTobik balintTobik commented Jan 16, 2025

- Description of the problem which is fixed/What is the use case
current default annotations doesn't include "image", it should be patched during operator deployment to be like:
enable_annotations = ["default_vcpus", "default_memory", "machine_type", "image", "default_gpus", "gpu_model" ]

Fixes: KATA-3493

- What I did
Extended hypervisor.remote's enable_annotation field with "image", "default_gpus" and "gpu_model" values.
Generated them to mc-40-kata-remote-config.yaml machineconfig.

- How to verify it
Operator should populate these new values to /opt/kata/configuration-remote.toml file on kata nodes.

- Description for the changelog
Add "image", "default_gpus" and "gpu_model" to enable_annotation field in configuration.toml and regenarate mc-40-kata-remote-config.yaml.

@openshift-ci-robot
Copy link

openshift-ci-robot commented Jan 16, 2025

@balintTobik: This pull request references KATA-3493 which is a valid jira issue.

In response to this:

- Description of the problem which is fixed/What is the use case
current default annotations doesn't include "image", it should be patched during operator deployment to be like:
enable_annotations = ["default_vcpus", "default_memory", "machine_type", "image", "default_gpus", "gpu_model" ]

Fixes: KATA-3493

- What I did
Extended hypervisor.remote's enable_annotation field with "image", "default_gpus" and "gpu_model" values.
Generated them to mc-40-kata-remote-config.yaml machineconfig.

- How to verify it
Operator should populate these new values to /opt/kata/configuration-remote.toml file on kata nodes.

- Description for the changelog
Add "image", "default_gpus" and "gpu_model" to enable_annotation field in configuration.toml and regenarate mc-40-kata-remote-config.yaml.

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 openshift-eng/jira-lifecycle-plugin repository.

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Jan 16, 2025
@openshift-ci-robot
Copy link

openshift-ci-robot commented Jan 16, 2025

@balintTobik: This pull request references KATA-3493 which is a valid jira issue.

In response to this:

- Description of the problem which is fixed/What is the use case
current default annotations doesn't include "image", it should be patched during operator deployment to be like:
enable_annotations = ["default_vcpus", "default_memory", "machine_type", "image", "default_gpus", "gpu_model" ]

Fixes: KATA-3493

- What I did
Extended hypervisor.remote's enable_annotation field with "image", "default_gpus" and "gpu_model" values.
Generated them to mc-40-kata-remote-config.yaml machineconfig.

- How to verify it
Operator should populate these new values to /opt/kata/configuration-remote.toml file on kata nodes.

- Description for the changelog
Add "image", "default_gpus" and "gpu_model" to enable_annotation field in configuration.toml and regenarate mc-40-kata-remote-config.yaml.

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 openshift-eng/jira-lifecycle-plugin repository.

@openshift-ci openshift-ci bot requested review from jensfr and vvoronko January 16, 2025 15:32
@openshift-ci openshift-ci bot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Jan 16, 2025
Copy link

openshift-ci bot commented Jan 16, 2025

Hi @balintTobik. Thanks for your PR.

I'm waiting for a openshift member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

Copy link
Member

@gkurz gkurz left a comment

Choose a reason for hiding this comment

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

Hi @balintTobik ! Thanks for working on that.

Overall change looks good. One issue though is that the PR contains a merge commit which is best avoided : we only want to see your changes there, not the various things you had to do in order to keep your PR in sync with the base branch.

The typical way of syncing pending work on top of the base branch is to git rebase in your local repo and force push to the PR. Please refresh your PR so that it just have the KATA-3493: update configuration-remote.toml commit (and address the newline nit on the way).

extend enable_annotations list with "image", "default_gpus" and
"gpu_model" and regenerate mc-40-kata-remote-config.yaml

Signed-off-by: Balint Tobik <[email protected]>
@gkurz
Copy link
Member

gkurz commented Jan 17, 2025

/ok-to-test

@openshift-ci openshift-ci bot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jan 17, 2025
Copy link

openshift-ci bot commented Jan 17, 2025

@balintTobik: all tests passed!

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.

Copy link
Member

@gkurz gkurz left a comment

Choose a reason for hiding this comment

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

/lgtm

Thanks @balintTobik !

@gkurz
Copy link
Member

gkurz commented Jan 17, 2025

Pre-merged tested by myself :

  • deployed this PR on OCP 4.17
  • created KataConfig with peer pods enabled
  • checked annotations are listed in kata configuration
$ oc debug --quiet node/worker0 -- chroot /host grep enable_annotations /opt/kata/configuration-remote.toml
enable_annotations = ["default_vcpus", "default_memory", "machine_type", "image", "default_gpus", "gpu_model"]

@gkurz gkurz merged commit a614f3d into openshift:devel Jan 17, 2025
4 checks passed
@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jan 17, 2025
@balintTobik balintTobik deleted the KATA-3493 branch January 20, 2025 14:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants