From 6ffbd32b40e6ad5f0718084fa2ec295f2fd3995d Mon Sep 17 00:00:00 2001 From: saffronjam Date: Sat, 1 Jun 2024 18:26:56 +0200 Subject: [PATCH 1/2] Fix worker status using wrong API url --- pkg/services/system_state_poll/service.go | 8 ++--- routers/api/v2/worker_status.go | 6 ++-- routers/router.go | 2 +- routers/routes/routes.go | 1 - routers/routes/status.go | 19 ------------ routers/routes/system.go | 2 ++ service/clients/v2.go | 1 - service/v2/api/api_client.go | 11 +++---- service/v2/client.go | 5 ---- service/v2/status/api_client.go | 30 ------------------- service/v2/status/opts/opts.go | 5 ---- service/v2/system/opts/opts.go | 4 +++ .../worker_service.go} | 4 +-- 13 files changed, 20 insertions(+), 78 deletions(-) delete mode 100644 routers/routes/status.go delete mode 100644 service/v2/status/api_client.go delete mode 100644 service/v2/status/opts/opts.go rename service/v2/{status/status_service.go => system/worker_service.go} (93%) diff --git a/pkg/services/system_state_poll/service.go b/pkg/services/system_state_poll/service.go index c2eee7e4..b37325ff 100644 --- a/pkg/services/system_state_poll/service.go +++ b/pkg/services/system_state_poll/service.go @@ -10,8 +10,8 @@ import ( func Setup(ctx context.Context) { log.Println("Starting pollers") - go services.PeriodicWorker(ctx, "statsWorker", StatsWorker, config.Config.Timer.FetchSystemStats) - go services.PeriodicWorker(ctx, "capacitiesWorker", CapacitiesWorker, config.Config.Timer.FetchSystemCapacities) - go services.PeriodicWorker(ctx, "statusWorker", StatusWorker, config.Config.Timer.FetchSystemStatus) - go services.PeriodicWorker(ctx, "gpuInfoWorker", GpuInfoWorker, config.Config.Timer.FetchSystemGpuInfo) + go services.PeriodicWorker(ctx, "systemStatsWorker", StatsWorker, config.Config.Timer.FetchSystemStats) + go services.PeriodicWorker(ctx, "systemCapacitiesWorker", CapacitiesWorker, config.Config.Timer.FetchSystemCapacities) + go services.PeriodicWorker(ctx, "systemStatusWorker", StatusWorker, config.Config.Timer.FetchSystemStatus) + go services.PeriodicWorker(ctx, "systemGpuInfoWorker", GpuInfoWorker, config.Config.Timer.FetchSystemGpuInfo) } diff --git a/routers/api/v2/worker_status.go b/routers/api/v2/worker_status.go index c8a9cfda..40168113 100644 --- a/routers/api/v2/worker_status.go +++ b/routers/api/v2/worker_status.go @@ -6,7 +6,7 @@ import ( "go-deploy/dto/v2/query" "go-deploy/pkg/sys" "go-deploy/service" - "go-deploy/service/v2/status/opts" + "go-deploy/service/v2/system/opts" ) // ListWorkerStatus @@ -18,7 +18,7 @@ import ( // @Success 200 {array} body.WorkerStatusRead // @Failure 400 {object} sys.ErrorResponse // @Failure 500 {object} sys.ErrorResponse -// @Router /v2/status [get] +// @Router /v2/workerStatus [get] func ListWorkerStatus(c *gin.Context) { context := sys.NewContext(c) @@ -28,7 +28,7 @@ func ListWorkerStatus(c *gin.Context) { return } - workerStatus, err := service.V2().Status().ListWorkerStatus(opts.ListWorkerStatusOpts{}) + workerStatus, err := service.V2().System().ListWorkerStatus(opts.ListWorkerStatusOpts{}) if err != nil { context.ServerError(err, InternalError) return diff --git a/routers/router.go b/routers/router.go index 167e98ec..66d8b323 100644 --- a/routers/router.go +++ b/routers/router.go @@ -52,7 +52,7 @@ func NewRouter() *gin.Engine { router.GET("/", func(c *gin.Context) { c.HTML(http.StatusOK, "index.html", gin.H{ "staticFolder": basePath + "/static", - "apiUrl": config.Config.ExternalUrl + "/v1/status", + "apiUrl": config.Config.ExternalUrl + "/v2/workerStatus", }) }) diff --git a/routers/routes/routes.go b/routers/routes/routes.go index e123e2ee..7638a117 100644 --- a/routers/routes/routes.go +++ b/routers/routes/routes.go @@ -34,7 +34,6 @@ func RoutingGroups() []RoutingGroup { ResourceMigrationRoutes(), SmRoutes(), SnapshotRoutes(), - StatusRoutes(), SystemRoutes(), TeamRoutes(), UserRoutes(), diff --git a/routers/routes/status.go b/routers/routes/status.go deleted file mode 100644 index a75f8e1d..00000000 --- a/routers/routes/status.go +++ /dev/null @@ -1,19 +0,0 @@ -package routes - -import v2 "go-deploy/routers/api/v2" - -const ( - StatusPath = "/v2/status" -) - -type StatusRoutingGroup struct{ RoutingGroupBase } - -func StatusRoutes() *StatusRoutingGroup { - return &StatusRoutingGroup{} -} - -func (group *StatusRoutingGroup) PublicRoutes() []Route { - return []Route{ - {Method: "GET", Pattern: StatusPath, HandlerFunc: v2.ListWorkerStatus}, - } -} diff --git a/routers/routes/system.go b/routers/routes/system.go index e7f87682..3a44e159 100644 --- a/routers/routes/system.go +++ b/routers/routes/system.go @@ -6,6 +6,7 @@ const ( SystemCapacitiesPath = "/v2/systemCapacities" SystemStatsPath = "/v2/systemStats" SystemStatusPath = "/v2/systemStatus" + WorkerStatusPath = "/v2/workerStatus" ) type SystemRoutingGroup struct{ RoutingGroupBase } @@ -19,5 +20,6 @@ func (group *SystemRoutingGroup) PublicRoutes() []Route { {Method: "GET", Pattern: SystemCapacitiesPath, HandlerFunc: v2.ListSystemCapacities}, {Method: "GET", Pattern: SystemStatsPath, HandlerFunc: v2.ListSystemStats}, {Method: "GET", Pattern: SystemStatusPath, HandlerFunc: v2.ListSystemStatus}, + {Method: "GET", Pattern: WorkerStatusPath, HandlerFunc: v2.ListWorkerStatus}, } } diff --git a/service/clients/v2.go b/service/clients/v2.go index 40947ab7..4b683342 100644 --- a/service/clients/v2.go +++ b/service/clients/v2.go @@ -15,7 +15,6 @@ type V2 interface { Jobs() apiV2.Jobs Notifications() apiV2.Notifications SMs() apiV2.SMs - Status() apiV2.Status Teams() apiV2.Teams Users() apiV2.Users ResourceMigrations() apiV2.ResourceMigrations diff --git a/service/v2/api/api_client.go b/service/v2/api/api_client.go index 7396ea7c..01c6a0ba 100644 --- a/service/v2/api/api_client.go +++ b/service/v2/api/api_client.go @@ -13,8 +13,7 @@ import ( resourceMigrationOpts "go-deploy/service/v2/resource_migrations/opts" smK8sService "go-deploy/service/v2/sms/k8s_service" smOpts "go-deploy/service/v2/sms/opts" - statusOpts "go-deploy/service/v2/status/opts" - zoneOpts "go-deploy/service/v2/system/opts" + systemOpts "go-deploy/service/v2/system/opts" teamOpts "go-deploy/service/v2/teams/opts" userOpts "go-deploy/service/v2/users/opts" vmK8sService "go-deploy/service/v2/vms/k8s_service" @@ -91,10 +90,6 @@ type SMs interface { K8s() *smK8sService.Client } -type Status interface { - ListWorkerStatus(opts ...statusOpts.ListWorkerStatusOpts) ([]model.WorkerStatus, error) -} - type Users interface { Get(id string, opts ...userOpts.GetOpts) (*model.User, error) GetByApiKey(apiKey string) (*model.User, error) @@ -195,11 +190,13 @@ type System interface { ListStats(n int) ([]body.TimestampedSystemStats, error) ListStatus(n int) ([]body.TimestampedSystemStatus, error) + ListWorkerStatus(opts ...systemOpts.ListWorkerStatusOpts) ([]model.WorkerStatus, error) + RegisterNode(params *body.HostRegisterParams) error ListHosts() ([]model.Host, error) GetZone(name string) *configModels.Zone - ListZones(opts ...zoneOpts.ListOpts) ([]configModels.Zone, error) + ListZones(opts ...systemOpts.ListOpts) ([]configModels.Zone, error) ZoneHasCapability(zoneName, capability string) bool } diff --git a/service/v2/client.go b/service/v2/client.go index 2f3c06a8..730b7b43 100644 --- a/service/v2/client.go +++ b/service/v2/client.go @@ -10,7 +10,6 @@ import ( "go-deploy/service/v2/notifications" "go-deploy/service/v2/resource_migrations" "go-deploy/service/v2/sms" - "go-deploy/service/v2/status" "go-deploy/service/v2/system" "go-deploy/service/v2/teams" "go-deploy/service/v2/users" @@ -70,10 +69,6 @@ func (c *Client) SMs() api.SMs { return sms.New(c, c.cache) } -func (c *Client) Status() api.Status { - return status.New(c, c.cache) -} - func (c *Client) System() api.System { return system.New(c, c.cache) } diff --git a/service/v2/status/api_client.go b/service/v2/status/api_client.go deleted file mode 100644 index 08632ec0..00000000 --- a/service/v2/status/api_client.go +++ /dev/null @@ -1,30 +0,0 @@ -package status - -import ( - "go-deploy/service/clients" - "go-deploy/service/core" -) - -// Client is the client for the Status service. -type Client struct { - // V2 is a reference to the parent client. - V2 clients.V2 - - // Cache is used to cache the resources fetched inside the service. - Cache *core.Cache -} - -// New creates a new Status service client. -func New(v2 clients.V2, cache ...*core.Cache) *Client { - var c *core.Cache - if len(cache) > 0 { - c = cache[0] - } else { - c = core.NewCache() - } - - return &Client{ - V2: v2, - Cache: c, - } -} diff --git a/service/v2/status/opts/opts.go b/service/v2/status/opts/opts.go deleted file mode 100644 index 9d709141..00000000 --- a/service/v2/status/opts/opts.go +++ /dev/null @@ -1,5 +0,0 @@ -package opts - -// ListWorkerStatusOpts is used to pass options to the List method -type ListWorkerStatusOpts struct { -} diff --git a/service/v2/system/opts/opts.go b/service/v2/system/opts/opts.go index ffffc36c..4a81ddb1 100644 --- a/service/v2/system/opts/opts.go +++ b/service/v2/system/opts/opts.go @@ -7,3 +7,7 @@ type GetOpts struct { // ListOpts is used to pass options to the List method type ListOpts struct { } + +// ListWorkerStatusOpts is used to pass options to the List method +type ListWorkerStatusOpts struct { +} diff --git a/service/v2/status/status_service.go b/service/v2/system/worker_service.go similarity index 93% rename from service/v2/status/status_service.go rename to service/v2/system/worker_service.go index d72d3183..dd84f2b0 100644 --- a/service/v2/status/status_service.go +++ b/service/v2/system/worker_service.go @@ -1,9 +1,9 @@ -package status +package system import ( "go-deploy/models/model" "go-deploy/pkg/db/resources/worker_status_repo" - "go-deploy/service/v2/status/opts" + "go-deploy/service/v2/system/opts" "time" ) From 5d4a0c8ed8392fbf98bec9e216d7496f29365331 Mon Sep 17 00:00:00 2001 From: saffronjam Date: Sat, 1 Jun 2024 18:54:36 +0200 Subject: [PATCH 2/2] Fix SM tests failing sometimes --- .github/workflows/run-tests.yml | 2 +- pkg/subsystems/k8s/job.go | 2 +- test/e2e/common.go | 6 ++++++ test/e2e/v2/sms/route_test.go | 7 +++---- 4 files changed, 11 insertions(+), 6 deletions(-) diff --git a/.github/workflows/run-tests.yml b/.github/workflows/run-tests.yml index 6c9d974d..0330ee52 100644 --- a/.github/workflows/run-tests.yml +++ b/.github/workflows/run-tests.yml @@ -37,7 +37,7 @@ jobs: - name: Run acceptance tests run: | - go test -timeout 30m ./test/acc/... + go test -timeout 90m ./test/acc/... - name: Start main program run: | diff --git a/pkg/subsystems/k8s/job.go b/pkg/subsystems/k8s/job.go index 1da319b0..2f52d48e 100644 --- a/pkg/subsystems/k8s/job.go +++ b/pkg/subsystems/k8s/job.go @@ -95,7 +95,7 @@ func (client *Client) CreateOneShotJob(public *models.JobPublic) error { } // Wait for the job to complete. - maxIter := 60 + maxIter := 600 iter := 0 for { k8sJob, err := client.K8sClient.BatchV1().Jobs(public.Namespace).Get(context.TODO(), public.Name, metav1.GetOptions{}) diff --git a/test/e2e/common.go b/test/e2e/common.go index 88d2395c..4a97bb40 100644 --- a/test/e2e/common.go +++ b/test/e2e/common.go @@ -253,6 +253,12 @@ func MustNotNil(t *testing.T, obj interface{}, msg string) { } } +func MustNotEmpty[T any](t *testing.T, slice []T, msg string) { + if len(slice) == 0 { + assert.FailNow(t, msg) + } +} + func IsUserError(code int) bool { return code >= 400 && code < 500 } diff --git a/test/e2e/v2/sms/route_test.go b/test/e2e/v2/sms/route_test.go index 29c6128b..e900d6c5 100644 --- a/test/e2e/v2/sms/route_test.go +++ b/test/e2e/v2/sms/route_test.go @@ -33,18 +33,17 @@ func TestList(t *testing.T) { func TestCreate(t *testing.T) { t.Parallel() - v2.ListDeployments(t, "?all=true") + v2.WithDeployment(t, body.DeploymentCreate{Name: e2e.GenName("sms-create")}, e2e.PowerUser) // Make sure the storage manager has time to be created - time.Sleep(5 * time.Second) + time.Sleep(30 * time.Second) storageManagers := v2.ListSMs(t, "?all=false") - assert.NotEmpty(t, storageManagers, "storage managers were empty") + e2e.MustNotEmpty(t, storageManagers, "storage managers were empty") storageManager := storageManagers[0] assert.NotEmpty(t, storageManager.ID, "storage manager id was empty") assert.NotEmpty(t, storageManager.OwnerID, "storage manager owner id was empty") - assert.NotEmpty(t, storageManager.URL, "storage manager url was empty") v2.WaitForSmRunning(t, storageManager.ID, func(storageManagerRead *body.SmRead) bool { // Make sure it is accessible