From 48c41a60f73361c9a2276a7fbbaf41fd4461281f Mon Sep 17 00:00:00 2001 From: JaeMan Park Date: Tue, 26 May 2026 14:41:53 +0900 Subject: [PATCH 1/2] Add waiting logic for host orchestrator settlement to backend for Docker While interacting with host orchestrator right after creating host API is returned, there is a possibility of getting 503 service temporarily unavailable error because host is created and booted but host orchestrator is not executed / ready yet. Handling logic for waiting host orchestrator readiness is currently in the client logic. Moving this logic into cloud orchestrator API endpoint makes the client simpler and reduces redundant API endpoints. Apply it first for Docker instance manager. Context: b/501288123 --- pkg/app/instances/docker.go | 12 +++++++ pkg/app/instances/helper.go | 50 ++++++++++++++++++++++++++ pkg/app/instances/helper_test.go | 62 ++++++++++++++++++++++++++++++++ pkg/app/instances/instances.go | 4 +++ 4 files changed, 128 insertions(+) create mode 100644 pkg/app/instances/helper.go create mode 100644 pkg/app/instances/helper_test.go diff --git a/pkg/app/instances/docker.go b/pkg/app/instances/docker.go index 1cdcd76c..23b0f843 100644 --- a/pkg/app/instances/docker.go +++ b/pkg/app/instances/docker.go @@ -188,6 +188,18 @@ func (m *DockerInstanceManager) waitCreateHostOperation(host string) (*apiv1.Hos return nil, fmt.Errorf("failed to inspect docker container: %w", err) } if res.State.Running { + client, err := m.GetHostClient("local", host) + if err != nil { + return nil, err + } + // There is a delay between host creation and its readiness, wait for host readiness and return when it is ready. + readinessChecker, ok := client.(HostReadinessChecker) + if !ok { + readinessChecker = &BasicHostReadinessChecker{Client: client} + } + if err := WaitForHostReady(readinessChecker, 2*time.Minute, 5*time.Second); err != nil { + return nil, err + } return &apiv1.HostInstance{ Name: host, }, nil diff --git a/pkg/app/instances/helper.go b/pkg/app/instances/helper.go new file mode 100644 index 00000000..1937d9f7 --- /dev/null +++ b/pkg/app/instances/helper.go @@ -0,0 +1,50 @@ +// Copyright 2026 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package instances + +import ( + "net/http" + "time" + + "github.com/google/cloud-android-orchestration/pkg/app/errors" +) + +func WaitForHostReady(checker HostReadinessChecker, maxWait time.Duration, retryDelay time.Duration) error { + deadline := time.Now().Add(maxWait) + + for time.Now().Before(deadline) { + ready, err := checker.IsHostReady() + if err != nil { + return err + } + if ready { + return nil + } + time.Sleep(retryDelay) + } + return errors.NewInternalError("wait for host orchestrator timed out", nil) +} + +type BasicHostReadinessChecker struct { + Client HostClient +} + +func (c *BasicHostReadinessChecker) IsHostReady() (bool, error) { + status, err := c.Client.Get("/", "", nil) + if err != nil || status == http.StatusBadGateway || status == http.StatusServiceUnavailable { + return false, nil + } + return true, nil +} diff --git a/pkg/app/instances/helper_test.go b/pkg/app/instances/helper_test.go new file mode 100644 index 00000000..d2ea21a8 --- /dev/null +++ b/pkg/app/instances/helper_test.go @@ -0,0 +1,62 @@ +// Copyright 2026 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package instances + +import ( + "testing" + "time" +) + +type mockHostReadinessChecker struct { + isHostReadyFunc func() (bool, error) +} + +func (c *mockHostReadinessChecker) IsHostReady() (bool, error) { + return c.isHostReadyFunc() +} + +func TestWaitForHostReadySucceeds(t *testing.T) { + checker := &mockHostReadinessChecker{ + isHostReadyFunc: func() (bool, error) { + return true, nil + }, + } + + err := WaitForHostReady(checker, 100*time.Millisecond, 1*time.Millisecond) + if err != nil { + t.Errorf("unexpected error: %v", err) + } +} + +func TestWaitForHostReadyRetries(t *testing.T) { + calls := 0 + checker := &mockHostReadinessChecker{ + isHostReadyFunc: func() (bool, error) { + calls++ + if calls == 1 { + return false, nil + } + return true, nil + }, + } + + err := WaitForHostReady(checker, 100*time.Millisecond, 1*time.Millisecond) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + if calls != 2 { + t.Errorf("expected 2 calls, got %d", calls) + } +} diff --git a/pkg/app/instances/instances.go b/pkg/app/instances/instances.go index c7095438..357726e5 100644 --- a/pkg/app/instances/instances.go +++ b/pkg/app/instances/instances.go @@ -38,6 +38,10 @@ type Manager interface { GetHostClient(zone string, host string) (HostClient, error) } +type HostReadinessChecker interface { + IsHostReady() (bool, error) +} + type HostClient interface { // Get and Post requests return the HTTP status code or an error. // The response body is parsed into the res output parameter if provided. From ced19d685edf9de35404499219a4c935e3b2bef0 Mon Sep 17 00:00:00 2001 From: JaeMan Park Date: Mon, 15 Jun 2026 14:36:13 +0900 Subject: [PATCH 2/2] Add waiting logic for host orchestrator settlement to backend for GCE While interacting with host orchestrator right after creating host API is returned, there is a possibility of getting 503 service temporarily unavailable error because host is created and booted but host orchestrator is not executed / ready yet. Handling logic for waiting host orchestrator readiness is currently in the client logic. Moving this logic into cloud orchestrator API endpoint makes the client simpler and reduces redundant API endpoints. Apply it for GCE instance manager. Context: b/501288123 --- pkg/app/instances/docker.go | 5 +- pkg/app/instances/gce.go | 114 +++++++++++++++++++++++++++++----- pkg/app/instances/gce_test.go | 19 +++++- pkg/app/instances/helper.go | 2 +- 4 files changed, 118 insertions(+), 22 deletions(-) diff --git a/pkg/app/instances/docker.go b/pkg/app/instances/docker.go index 23b0f843..c3ba4b3a 100644 --- a/pkg/app/instances/docker.go +++ b/pkg/app/instances/docker.go @@ -193,10 +193,7 @@ func (m *DockerInstanceManager) waitCreateHostOperation(host string) (*apiv1.Hos return nil, err } // There is a delay between host creation and its readiness, wait for host readiness and return when it is ready. - readinessChecker, ok := client.(HostReadinessChecker) - if !ok { - readinessChecker = &BasicHostReadinessChecker{Client: client} - } + readinessChecker := &BasicHostReadinessChecker{Client: client} if err := WaitForHostReady(readinessChecker, 2*time.Minute, 5*time.Second); err != nil { return nil, err } diff --git a/pkg/app/instances/gce.go b/pkg/app/instances/gce.go index d2304ebc..7f78ff93 100644 --- a/pkg/app/instances/gce.go +++ b/pkg/app/instances/gce.go @@ -18,9 +18,11 @@ import ( "context" "fmt" "log" + "net/http" "net/url" "path" "regexp" + "time" apiv1 "github.com/google/cloud-android-orchestration/api/v1" "github.com/google/cloud-android-orchestration/pkg/app/accounts" @@ -79,27 +81,19 @@ func (m *GCEInstanceManager) ListZones() (*apiv1.ListZonesResponse, error) { } func (m *GCEInstanceManager) GetHostAddr(zone string, host string) (string, error) { - instance, err := m.getHostInstance(zone, host) + ins, err := m.getHostInstance(zone, host) if err != nil { return "", err } - ilen := len(instance.NetworkInterfaces) - if ilen == 0 { - log.Printf("host instance %s in zone %s is missing a network interface", host, zone) - return "", errors.NewInternalError("host instance missing a network interface", nil) - } - if ilen > 1 { - log.Printf("host instance %s in zone %s has %d network interfaces", host, zone, ilen) - } - return instance.NetworkInterfaces[0].NetworkIP, nil + return m.getHostAddrWithIns(ins) } func (m *GCEInstanceManager) GetHostURL(zone string, host string) (*url.URL, error) { - addr, err := m.GetHostAddr(zone, host) + ins, err := m.getHostInstance(zone, host) if err != nil { return nil, err } - return url.Parse(fmt.Sprintf("%s://%s:%d", m.Config.HostOrchestratorProtocol, addr, m.Config.GCP.HostOrchestratorPort)) + return m.getHostURLWithIns(ins) } const operationStatusDone = "DONE" @@ -253,18 +247,105 @@ func (m *GCEInstanceManager) WaitOperation(zone string, user accounts.User, name if op.Status != operationStatusDone { return nil, errors.NewServiceUnavailableError("Wait for operation timed out", nil) } - getter := opResultGetter{Service: m.Service, Op: op} - return getter.Get() + getter := opResultGetter{ + Service: m.Service, + Op: op, + Config: &m.Config, + } + res, err := getter.Get() + if err != nil { + return nil, err + } + if hostInst, ok := res.(*apiv1.HostInstance); ok && op.OperationType == "insert" { + return m.waitHostAvailability(zone, user, hostInst.Name) + } + return res, nil } -func (m *GCEInstanceManager) GetHostClient(zone string, host string) (HostClient, error) { - url, err := m.GetHostURL(zone, host) +func (m *GCEInstanceManager) waitHostAvailability(zone string, user accounts.User, host string) (*apiv1.HostInstance, error) { + ins, err := m.getHostInstance(zone, host) + if err != nil { + return nil, err + } + hostInstance, err := BuildHostInstance(ins) + if err != nil { + return nil, err + } + client, err := m.getHostClientWithIns(ins) + if err != nil { + return nil, err + } + if err := m.waitForOrchestrator(zone, hostInstance, client); err != nil { + return nil, err + } + return hostInstance, nil +} + +type gceHostReadinessChecker struct { + basicChecker *BasicHostReadinessChecker + manager *GCEInstanceManager + zone string + hostName string +} + +func (g *gceHostReadinessChecker) IsHostReady() (bool, error) { + _, err := g.manager.Service.Instances.Get(g.manager.Config.GCP.ProjectID, g.zone, g.hostName).Context(context.TODO()).Do() + if err != nil { + if apiErr, ok := err.(*googleapi.Error); ok && apiErr.Code == http.StatusNotFound { + return false, errors.NewNotFoundError("Host was deleted concurrently", err) + } + return false, fmt.Errorf("failed to check host existence: %w", err) + } + return g.basicChecker.IsHostReady() +} + +func (m *GCEInstanceManager) waitForOrchestrator(zone string, host *apiv1.HostInstance, client HostClient) error { + gceChecker := &gceHostReadinessChecker{ + basicChecker: &BasicHostReadinessChecker{Client: client}, + manager: m, + zone: zone, + hostName: host.Name, + } + + return WaitForHostReady(gceChecker, 5*time.Minute, 5*time.Second) +} + +func (m *GCEInstanceManager) getHostAddrWithIns(ins *compute.Instance) (string, error) { + ilen := len(ins.NetworkInterfaces) + if ilen == 0 { + log.Printf("host instance %s in zone %s is missing a network interface", ins.Name, ins.Zone) + return "", errors.NewInternalError("host instance missing a network interface", nil) + } + if ilen > 1 { + log.Printf("host instance %s in zone %s has %d network interfaces", ins.Name, ins.Zone, ilen) + } + return ins.NetworkInterfaces[0].NetworkIP, nil +} + +func (m *GCEInstanceManager) getHostURLWithIns(ins *compute.Instance) (*url.URL, error) { + addr, err := m.getHostAddrWithIns(ins) + if err != nil { + return nil, err + } + return url.Parse(fmt.Sprintf("%s://%s:%d", m.Config.HostOrchestratorProtocol, addr, m.Config.GCP.HostOrchestratorPort)) +} + +func (m *GCEInstanceManager) getHostClientWithIns(ins *compute.Instance) (HostClient, error) { + url, err := m.getHostURLWithIns(ins) if err != nil { return nil, err } return NewNetHostClient(url, m.Config.AllowSelfSignedHostSSLCertificate), nil } +func (m *GCEInstanceManager) GetHostClient(zone string, host string) (HostClient, error) { + ins, err := m.getHostInstance(zone, host) + if err != nil { + return nil, err + } + return m.getHostClientWithIns(ins) +} + func (m *GCEInstanceManager) getHostInstance(zone string, host string) (*compute.Instance, error) { ins, err := m.Service.Instances. Get(m.Config.GCP.ProjectID, zone, host). @@ -326,6 +407,7 @@ var ( type opResultGetter struct { Service *compute.Service Op *compute.Operation + Config *Config } func (g *opResultGetter) Get() (any, error) { diff --git a/pkg/app/instances/gce_test.go b/pkg/app/instances/gce_test.go index 18c55693..98f6e6b2 100644 --- a/pkg/app/instances/gce_test.go +++ b/pkg/app/instances/gce_test.go @@ -24,6 +24,7 @@ import ( "net/http/httptest" "net/url" "reflect" + "strconv" "strings" "testing" @@ -572,9 +573,14 @@ func TestWaitCreateInstanceOperationSucceeds(t *testing.T) { Name: "foo", MachineType: "mt", MinCpuPlatform: "mcp", + NetworkInterfaces: []*compute.NetworkInterface{ + {NetworkIP: "127.0.0.1"}, + }, } ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { switch path := r.URL.Path; path { + case "/": + replyJSON(w, map[string]string{}) case "/projects/google.com:test-project/zones/us-central1-a/operations/operation-1/wait": replyJSON(w, operation) case "/projects/google.com:test-project/zones/us-central1-a/instances/foo": @@ -584,7 +590,18 @@ func TestWaitCreateInstanceOperationSucceeds(t *testing.T) { } })) defer ts.Close() - im := NewGCEInstanceManager(testConfig, buildTestService(t, ts), testNameGenerator) + + tsURL, _ := url.Parse(ts.URL) + port, _ := strconv.Atoi(tsURL.Port()) + + cfg := testConfig + cfg.HostOrchestratorProtocol = "http" + + gcpCfg := *cfg.GCP + gcpCfg.HostOrchestratorPort = port + cfg.GCP = &gcpCfg + + im := NewGCEInstanceManager(cfg, buildTestService(t, ts), testNameGenerator) res, _ := im.WaitOperation(zone, &TestUser{}, opName) diff --git a/pkg/app/instances/helper.go b/pkg/app/instances/helper.go index 1937d9f7..7d31d50d 100644 --- a/pkg/app/instances/helper.go +++ b/pkg/app/instances/helper.go @@ -34,7 +34,7 @@ func WaitForHostReady(checker HostReadinessChecker, maxWait time.Duration, retry } time.Sleep(retryDelay) } - return errors.NewInternalError("wait for host orchestrator timed out", nil) + return errors.NewInternalError("wait for host orchestrator readiness timed out", nil) } type BasicHostReadinessChecker struct {