CNV-32684: Fixing the dynamic ssh key injection #1538
Conversation
|
@DanaOrr: This pull request references CNV-32684 which is a valid jira issue. In response to this:
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/test-infra repository. |
|
@DanaOrr: This pull request references CNV-32684 which is a valid jira issue. In response to this:
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/test-infra repository. |
|
/retest |
1 similar comment
|
/retest |
| ): V1beta1DataVolumeSourcePVC | V1beta1DataVolumeSourceRef => { | ||
| const bootDisk = getBootDisk(vm); | ||
| const volume = getVolumes(vm)?.find((vol) => vol.name === bootDisk?.name); | ||
| const dataVolumeTemplate = vm?.spec?.dataVolumeTemplates?.find( |
There was a problem hiding this comment.
instead of vm?.spec?.dataVolumeTemplates please use the selector function getDataVolumeTemplates
| const bootDisk = getBootDisk(vm); | ||
| const volume = getVolumes(vm)?.find((vol) => vol.name === bootDisk?.name); | ||
| const dataVolumeTemplate = vm?.spec?.dataVolumeTemplates?.find( | ||
| (dv) => dv.metadata?.name === volume?.dataVolume?.name, |
There was a problem hiding this comment.
instead of using ?.metadata?.name use selector function getName for dv and volume
| } | ||
|
|
||
| if (dataVolumeTemplate?.spec?.source?.pvc) { | ||
| return dataVolumeTemplate?.spec?.source?.pvc; |
There was a problem hiding this comment.
consider returning something like:
{
name: pvc?.name
namespace: pvc?.namespace
kind: PersistentVolumeClaimModel.kind
}
this will allow you to get a specific resource in your hook useDynamicSSHInjectionEnabledForVM
(no need to fetch all volumes, and find the specific one in the hook)
| const isDynamicSSHInjectionTemplate = useMemo(() => getIsDynamicSSHInjectionEnabled(vm), [vm]); | ||
| const isDynamicSSHInjectionInstanceType = useDynamicSSHInjectionEnabledForVM(vm); |
There was a problem hiding this comment.
both constants are used together wherever they are used, combine the getIsDynamicSSHInjectionEnabled function with the useDynamicSSHInjectionEnabledForVM hook.
Also useDynamicSSHInjection would be fine as a name for the hook if you want a shorter one
| const isDynamicSSHInjectionEnabled = | ||
| getLabel(bootableVolume, DYNAMIC_CREDENTIALS_SUPPORT) === 'true' && | ||
| Boolean(getAccessCredentials(vm)?.[0]?.sshPublicKey?.propagationMethod?.qemuGuestAgent); |
There was a problem hiding this comment.
this code is very similar to getIsDynamicSSHInjectionEnabled, can you make getIsDynamicSSHInjectionEnabled to get any kind of K8s resource and not just vm?
DanaOrr
force-pushed
the
fixinig-dynamic-ssh-key-injection
branch
from
192a7b3
to
159bd5f
Compare
Signed-off-by: Dana Orr <dorr@redhat.com>
DanaOrr
force-pushed
the
fixinig-dynamic-ssh-key-injection
branch
from
159bd5f
to
2828997
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: avivtur, DanaOrr The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/cherry-pick release-4.14 |
|
@DanaOrr: new pull request created: #1574 In response to this:
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/test-infra repository. |
📝 Description
These changes involve adding the missing line - 'runcmd - [ setsebool, -P, virt_qemu_ga_manage_ssh, on ]` to the YAML in instanceTypes.
Also, I fixed the problem that after creating a VM, and the dynamic SSH key injection switch was turned on, we could not edit the ssh key. Now, we should be able to edit or remove the SSH key.
If the dynamic SSH key injection switch has not been activated, it will not be possible to edit the ssh key after creation and the option to edit will be disabled.
🎥 Demo
Template-
InstanceType -
