Skip to content
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

test: verify multiplatform cleanup #960

Merged

Conversation

stuartwdouglas
Copy link
Contributor

@stuartwdouglas stuartwdouglas commented Dec 14, 2023

This is needed for STONEBLD-1779 to verify we are not leaving credentials on the remote host

Description

Verifies that after a multi platform build the cleanup is successful.

Issue ticket number and link

https://issues.redhat.com/browse/STONEBLD-1779

@stuartwdouglas stuartwdouglas force-pushed the multiplatform-cleanup branch 4 times, most recently from 91323fc to e40a687 Compare December 15, 2023 04:58
@stuartwdouglas
Copy link
Contributor Author

/retest

@mmorhun
Copy link
Contributor

mmorhun commented Dec 15, 2023

/lgtm

Comment on lines +212 to +216
userDir = string(userDirTmp)
hostParts := strings.Split(string(fullHost), "@")
host = strings.TrimSpace(hostParts[1])
})
Copy link
Contributor

Choose a reason for hiding this comment

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

Was there supposed to be some check for the userDir and host?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no, we use the host for the SSH connection, then verify that the userDir does not exist as it has been cleaned up.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, ok. It looks very wrong, could you at least add a comment that these are set here only so that the other It node can use them?

@stuartwdouglas
Copy link
Contributor Author

/retest

@stuartwdouglas
Copy link
Contributor Author

Another failure in an unrelated test suite

@flacatus
Copy link
Collaborator

/retest

This is needed for STONEBLD-1779 to verify we are not leaving
credentials on the remote host
@openshift-ci openshift-ci bot removed the lgtm label Dec 17, 2023
Copy link

openshift-ci bot commented Dec 17, 2023

New changes are detected. LGTM label has been removed.

Copy link

Quality Gate Passed Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
0.3% Duplication on New Code

See analysis details on SonarCloud

Copy link

openshift-ci bot commented Dec 17, 2023

[APPROVALNOTIFIER] This PR is APPROVED

Approval requirements bypassed by manually added approval.

This pull-request has been approved by: mmorhun

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-bot openshift-merge-bot bot merged commit 0d03613 into konflux-ci:main Dec 17, 2023
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants