Skip to content

fix: update vsphere-supervisor test to skip output from retry (follow-up) #323

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.

Sign up for GitHub

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

Jump to bottom
Merged
nywilken merged 1 commit into from
Nov 9, 2023
Merged

fix: update vsphere-supervisor test to skip output from retry (follow-up) #323

nywilken merged 1 commit into from
Nov 9, 2023

Conversation

@dilyar85
Copy link
Contributor

@dilyar85 dilyar85 commented Nov 9, 2023

This is a follow-up change to #321 which appears still failing the CI test.

To verify, I manually updated the test code to ensure the timing issue occurs:

--- a/builder/vsphere/supervisor/step_watch_source_test.go
+++ b/builder/vsphere/supervisor/step_watch_source_test.go
@@ -116,6 +116,8 @@ func TestWatchSource_Run(t *testing.T) {
        vmObj.Status.VmIp = testVMIP
        _ = kubeClient.Update(ctx, vmObj, opt)

+       time.Sleep(time.Second)
+
        vmServiceObj.Status.LoadBalancer.Ingress[0].IP = testIngressIP
        _ = kubeClient.Update(ctx, vmServiceObj, opt)

Running the above test against upstream/main:

$ go test -count=1 -run ^TestWatchSource_Run$ github.com/hashicorp/packer-plugin-vsphere/builder/vsphere/supervisor
2023/11/09 10:38:35 ui: Waiting for the source VM to be powered-on and accessible...
2023/11/09 10:38:36 ui: Source VM is NOT powered-on yet, continue watching...
2023/11/09 10:38:36 ui: Source VM is powered-on, waiting for an IP to be assigned...
2023/11/09 10:38:36 ui: Successfully obtained the source VM IP: 1.2.3.4
2023/11/09 10:38:36 ui: Getting source VM ingress IP from the VMService object
2023/11/09 10:38:36 ui: VMService object's ingress IP is empty, continue checking...
2023/11/09 10:38:36 Retryable error: VMService object's ingress IP is empty, continue checking...
2023/11/09 10:38:41 ui: Successfully retrieved the source VM ingress IP: 5.6.7.8
2023/11/09 10:38:41 ui: Source VM is now ready in Supervisor cluster
--- FAIL: TestWatchSource_Run (6.00s)
    utils_test.go:61: Expected output "Successfully retrieved the source VM ingress IP: 5.6.7.8" but got "VMService object's ingress IP is empty, continue checking..."
FAIL
FAIL	github.com/hashicorp/packer-plugin-vsphere/builder/vsphere/supervisor	6.748s
FAIL

Running the above test against this PR:

$ go test -count=1 -run ^TestWatchSource_Run$ github.com/hashicorp/packer-plugin-vsphere/builder/vsphere/supervisor
ok  	github.com/hashicorp/packer-plugin-vsphere/builder/vsphere/supervisor	6.587s

@dilyar85 dilyar85 requested a review from a team as a code owner November 9, 2023 15:49
nywilken
nywilken reviewed Nov 9, 2023
View reviewed changes
builder/vsphere/supervisor/utils_test.go
// Skip "continue checking" line as it can be printed from the retry.
if strings.Contains(actual, "continue checking") {
readLine(t, writer)
return readLine(t, writer)
Copy link
Contributor

@nywilken nywilken Nov 9, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Ha I was looking at this line in the previous PR and thought do we return properly here. Good catch.

Copy link
Contributor Author

@dilyar85 dilyar85 Nov 9, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I think I just have some muscle memory that avoids recursions as much as possible :)

@tenthirtyam tenthirtyam added chore Chore builder/vsphere-supervisor Builder: vsphere-supervisor labels Nov 9, 2023
nywilken
nywilken approved these changes Nov 9, 2023
View reviewed changes
Copy link
Contributor

@nywilken nywilken left a comment

Choose a reason for hiding this comment

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

LGTM

@nywilken nywilken merged commit dfe0677 into hashicorp:main Nov 9, 2023
@hashicorp hashicorp locked and limited conversation to collaborators Jun 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@nywilken nywilken nywilken approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@dilyar85 dilyar85

Labels

builder/vsphere-supervisor Builder: vsphere-supervisor chore Chore

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@dilyar85 @nywilken @tenthirtyam