From d03a6200098af29302990ef7db89b93c3eccfca5 Mon Sep 17 00:00:00 2001 From: Charles Treatman Date: Tue, 30 Jan 2024 12:39:24 -0600 Subject: [PATCH 1/4] fix: handle optional flags correctly in VRF route update --- internal/vrf/updateroute.go | 14 ++++++++++---- test/e2e/vrfstest/vrf_route_test.go | 25 +++++++++++-------------- test/helper/helper.go | 27 +++++++++++++++++++++++++++ 3 files changed, 48 insertions(+), 18 deletions(-) diff --git a/internal/vrf/updateroute.go b/internal/vrf/updateroute.go index 3aa79486..d508bd53 100644 --- a/internal/vrf/updateroute.go +++ b/internal/vrf/updateroute.go @@ -29,10 +29,16 @@ func (c *Client) UpdateRoute() *cobra.Command { inc := []string{} exc := []string{} - vrfRouteUpdateInput := metalv1.VrfRouteUpdateInput{ - Prefix: &prefix, - NextHop: &nextHop, - Tags: tags, + vrfRouteUpdateInput := metalv1.VrfRouteUpdateInput{} + + if prefix != "" { + vrfRouteUpdateInput.Prefix = &prefix + } + if nextHop != "" { + vrfRouteUpdateInput.NextHop = &nextHop + } + if cmd.Flag("tags").Changed { + vrfRouteUpdateInput.Tags = tags } vrfRoute, _, err := c.Service.UpdateVrfRouteById(context.Background(), vrfID).VrfRouteUpdateInput(vrfRouteUpdateInput).Include(c.Servicer.Includes(inc)).Exclude(c.Servicer.Excludes(exc)).Execute() diff --git a/test/e2e/vrfstest/vrf_route_test.go b/test/e2e/vrfstest/vrf_route_test.go index 76d18bcc..c0f99daf 100644 --- a/test/e2e/vrfstest/vrf_route_test.go +++ b/test/e2e/vrfstest/vrf_route_test.go @@ -5,6 +5,7 @@ import ( "strings" "testing" + "github.com/equinix/equinix-sdk-go/services/metalv1" root "github.com/equinix/metal-cli/internal/cli" outputPkg "github.com/equinix/metal-cli/internal/outputs" "github.com/equinix/metal-cli/internal/vrf" @@ -113,10 +114,6 @@ func TestCli_Vrf_Route(t *testing.T) { }, want: &cobra.Command{}, cmdFunc: func(t *testing.T, c *cobra.Command) { - // Actually user have to wait for 5 min to updae the VRF-routes. This test case is skipped intentionally - if true { - t.Skip("Skipping this test because someCondition is true") - } root := c.Root() projName := "metal-cli-" + randName + "-vrf-list-test" @@ -128,19 +125,19 @@ func TestCli_Vrf_Route(t *testing.T) { ipReservation := helper.CreateTestVrfIpRequest(t, projectId.GetId(), vrf.GetId()) _ = helper.CreateTestVrfGateway(t, projectId.GetId(), ipReservation.VrfIpReservation.GetId(), vlan.GetId()) - _ = helper.CreateTestVrfRoute(t, vrf.GetId()) + route := helper.CreateTestVrfRoute(t, vrf.GetId()) - if vlan.GetId() != "" && vrf.GetId() != "" { - root.SetArgs([]string{subCommand, "update-route", "-i", vrf.GetId(), "-t", "foobar"}) + _ = helper.WaitForVrfRouteState(t, route.GetId(), metalv1.VRFROUTESTATUS_ACTIVE) - out := helper.ExecuteAndCaptureOutput(t, root) + root.SetArgs([]string{subCommand, "update-route", "-i", route.GetId(), "-t", "foobar"}) - if !strings.Contains(string(out[:]), "TYPE") && - !strings.Contains(string(out[:]), "static") && - !strings.Contains(string(out[:]), "PREFIX") && - !strings.Contains(string(out[:]), "0.0.0.0/0") { - t.Error("expected output should include TYPE static PREFIX and 0.0.0.0/0, in the out string ") - } + out := helper.ExecuteAndCaptureOutput(t, root) + + if !strings.Contains(string(out[:]), "TYPE") && + !strings.Contains(string(out[:]), "static") && + !strings.Contains(string(out[:]), "PREFIX") && + !strings.Contains(string(out[:]), "0.0.0.0/0") { + t.Error("expected output should include TYPE static PREFIX and 0.0.0.0/0, in the out string ") } } }, diff --git a/test/helper/helper.go b/test/helper/helper.go index 1d7b4e84..591c97d4 100644 --- a/test/helper/helper.go +++ b/test/helper/helper.go @@ -197,6 +197,33 @@ func WaitForDeviceState(t *testing.T, deviceId string, states ...metalv1.DeviceS return false, fmt.Errorf("timed out waiting for device %v state %v to become one of %v", deviceId, device.GetState(), states) } +func WaitForVrfRouteState(t *testing.T, routeId string, statuses ...metalv1.VrfRouteStatus) bool { + var route *metalv1.VrfRoute + var err error + t.Helper() + predefinedTime := 900 * time.Second // Adjust this as needed + retryInterval := 10 * time.Second // Adjust this as needed + startTime := time.Now() + client := TestClient() + for time.Since(startTime) < predefinedTime { + route, _, err = client.VRFsApi.FindVrfRouteById(context.Background(), routeId).Execute() + if err != nil { + t.Fatal(err) + return false + } + for _, status := range statuses { + if route.GetStatus() == status { + return true + } + } + + // Sleep for the specified interval + time.Sleep(retryInterval) + } + t.Fatalf("timed out waiting for VRF route %v status %v to become one of %v", routeId, route.GetStatus(), statuses) + return false +} + func WaitForAttachVlanToPort(t *testing.T, portId string, attach bool) error { t.Helper() ticker := time.NewTicker(5 * time.Second) From ce58bea836058e0e24bcc1ea6b7f4ca7a8739f56 Mon Sep 17 00:00:00 2001 From: Charles Treatman Date: Wed, 31 Jan 2024 11:03:12 -0600 Subject: [PATCH 2/4] hard-code a 5-minute wait because that's what the API does --- test/e2e/vrfstest/vrf_route_test.go | 7 +++++-- test/helper/helper.go | 29 +---------------------------- 2 files changed, 6 insertions(+), 30 deletions(-) diff --git a/test/e2e/vrfstest/vrf_route_test.go b/test/e2e/vrfstest/vrf_route_test.go index c0f99daf..9b58e704 100644 --- a/test/e2e/vrfstest/vrf_route_test.go +++ b/test/e2e/vrfstest/vrf_route_test.go @@ -4,8 +4,8 @@ import ( "regexp" "strings" "testing" + "time" - "github.com/equinix/equinix-sdk-go/services/metalv1" root "github.com/equinix/metal-cli/internal/cli" outputPkg "github.com/equinix/metal-cli/internal/outputs" "github.com/equinix/metal-cli/internal/vrf" @@ -127,7 +127,10 @@ func TestCli_Vrf_Route(t *testing.T) { _ = helper.CreateTestVrfGateway(t, projectId.GetId(), ipReservation.VrfIpReservation.GetId(), vlan.GetId()) route := helper.CreateTestVrfRoute(t, vrf.GetId()) - _ = helper.WaitForVrfRouteState(t, route.GetId(), metalv1.VRFROUTESTATUS_ACTIVE) + // We literally need to sleep for 5 minutes; the API will reject any + // VRF route update request that comes in less than 5 minutes after + // the VRF route was last updated + time.Sleep(300 * time.Second) root.SetArgs([]string{subCommand, "update-route", "-i", route.GetId(), "-t", "foobar"}) diff --git a/test/helper/helper.go b/test/helper/helper.go index 591c97d4..0fd229bc 100644 --- a/test/helper/helper.go +++ b/test/helper/helper.go @@ -28,7 +28,7 @@ func TestClient() *metalv1.APIClient { configuration.AddDefaultHeader("X-Auth-Token", os.Getenv("METAL_AUTH_TOKEN")) configuration.UserAgent = fmt.Sprintf("metal-cli/test-helper %s", configuration.UserAgent) // For debug purpose - // configuration.Debug = true + configuration.Debug = true apiClient := metalv1.NewAPIClient(configuration) return apiClient } @@ -197,33 +197,6 @@ func WaitForDeviceState(t *testing.T, deviceId string, states ...metalv1.DeviceS return false, fmt.Errorf("timed out waiting for device %v state %v to become one of %v", deviceId, device.GetState(), states) } -func WaitForVrfRouteState(t *testing.T, routeId string, statuses ...metalv1.VrfRouteStatus) bool { - var route *metalv1.VrfRoute - var err error - t.Helper() - predefinedTime := 900 * time.Second // Adjust this as needed - retryInterval := 10 * time.Second // Adjust this as needed - startTime := time.Now() - client := TestClient() - for time.Since(startTime) < predefinedTime { - route, _, err = client.VRFsApi.FindVrfRouteById(context.Background(), routeId).Execute() - if err != nil { - t.Fatal(err) - return false - } - for _, status := range statuses { - if route.GetStatus() == status { - return true - } - } - - // Sleep for the specified interval - time.Sleep(retryInterval) - } - t.Fatalf("timed out waiting for VRF route %v status %v to become one of %v", routeId, route.GetStatus(), statuses) - return false -} - func WaitForAttachVlanToPort(t *testing.T, portId string, attach bool) error { t.Helper() ticker := time.NewTicker(5 * time.Second) From dd6c940015947c3348c2f2b9bd7df4d005fbd853 Mon Sep 17 00:00:00 2001 From: Charles Treatman Date: Wed, 31 Jan 2024 11:03:44 -0600 Subject: [PATCH 3/4] increase go test timeout to match terraform so that individual tests can time out instead of panic --- .github/workflows/e2e-test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/e2e-test.yml b/.github/workflows/e2e-test.yml index 9d355d28..4bedbb4f 100644 --- a/.github/workflows/e2e-test.yml +++ b/.github/workflows/e2e-test.yml @@ -48,6 +48,6 @@ jobs: - name: Get dependencies run: go mod download - name: Run end-to-end tests - run: go test -v -cover -coverpkg=./... -parallel 4 ./test/e2e/... -timeout 1000s + run: go test -v -cover -coverpkg=./... -parallel 4 ./test/e2e/... -timeout 180m env: METAL_AUTH_TOKEN: ${{ secrets.METAL_AUTH_TOKEN }} From 49b48269c1473cfaaf083aa00395b5d969f86fe9 Mon Sep 17 00:00:00 2001 From: Charles Treatman Date: Wed, 31 Jan 2024 15:06:08 -0600 Subject: [PATCH 4/4] disable test debug output again --- test/helper/helper.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/helper/helper.go b/test/helper/helper.go index 0fd229bc..1d7b4e84 100644 --- a/test/helper/helper.go +++ b/test/helper/helper.go @@ -28,7 +28,7 @@ func TestClient() *metalv1.APIClient { configuration.AddDefaultHeader("X-Auth-Token", os.Getenv("METAL_AUTH_TOKEN")) configuration.UserAgent = fmt.Sprintf("metal-cli/test-helper %s", configuration.UserAgent) // For debug purpose - configuration.Debug = true + // configuration.Debug = true apiClient := metalv1.NewAPIClient(configuration) return apiClient }