WIP: Core libraries for two node disruptive tests. #30332
Conversation
openshift-ci
bot
added
the
do-not-merge/work-in-progress
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: jaypoulz 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 |
|
|
||
| // VerifyHypervisorConnectivity verifies SSH connectivity to the hypervisor and checks | ||
| // that virsh and libvirt are available. | ||
| func VerifyHypervisorConnectivity(sshConfig *SSHConfig, knownHostsPath string) error { |
There was a problem hiding this comment.
Would rename to VerifyHypervisor or VerifyHypervisorAvailability (as you're checking for more than connectivity and there is already a VerifyConnectivity function)
|
|
||
| // SSH to hypervisor, then to surviving node to run pcs debug-start | ||
| // We need to chain the SSH commands: host -> hypervisor -> surviving node | ||
| output, stderr, err := PcsCommand(fmt.Sprintf("%s && %s", pcsResourceDebugStop, formatPcsCommandString(pcsResourceDebugStart, pcsResourceDebugStartEnvVars)), sshConfig, localKnownHostsPath, remoteKnownHostsPath, nodeIP) |
There was a problem hiding this comment.
I think the inconsistency here between having a "formatPcsCommandString" for the second command and directly calling fmt.Sprintf for the first one (in the first parameter) makes this a little harder to read than it should. Is it worth also encapsulating the first one in a function?
There was a problem hiding this comment.
Reading the code further, why aren't we doing this like the PcsDebugStart below, that uses ExecuteRemoteSSHCommand? That's much easier to parse
There was a problem hiding this comment.
I just removed the utility function and replaced all of the pcs commands with the formatPcsCommand string option for simplicity :)
test/extended/two_node/utils/ssh.go
Outdated
|
|
||
| // ExecuteRemoteSSHCommand executes a command on an OpenShift node via two-hop SSH (local → hypervisor → node). | ||
| // Uses 'core' user for the node connection. | ||
| func ExecuteRemoteSSHCommand(nodeIP, command string, sshConfig *SSHConfig, localKnownHostsPath, remoteKnownHostsPath string) (string, string, error) { |
There was a problem hiding this comment.
I would rename nodeIP to remoteNodeIP, to make it more explicit. Also sshConfig to hypervisorSSHConfig. This way it's easier to know what info each parameter is providing to the function
jaypoulz
force-pushed
the
two-node-disruption-test-libs
branch
4 times, most recently
from
3bde7a8 to
323411e
Compare
jaypoulz
force-pushed
the
two-node-disruption-test-libs
branch
from
323411e to
ef0ff06
Compare
|
@jaypoulz: The following tests failed, say
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. |
|
Job Failure Risk Analysis for sha: ef0ff06
|
No description provided.