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

Author: k311093

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 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 getHostURLWithIns(ins, &m.Config)
 }
 
 const operationStatusDone = "DONE"
@@ -253,16 +247,108 @@ 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) 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 := getHostClientWithIns(ins, &m.Config)
+	if err != nil {
+		return nil, err
+	}
+	if err := m.waitForOrchestrator(zone, hostInstance, client); err != nil {
+		return nil, err
+	}
+	return hostInstance, nil
+}
+
+type gceReadinessChecker struct {
+	checker  ReadinessChecker
+	manager  *GCEInstanceManager
+	zone     string
+	hostName string
+}
+
+func (g *gceReadinessChecker) 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.checker.IsHostReady()
+}
+
+func (m *GCEInstanceManager) waitForOrchestrator(zone string, host *apiv1.HostInstance, client HostClient) error {
+	readinessChecker, ok := client.(ReadinessChecker)
+	if !ok {
+		readinessChecker = &defaultReadinessChecker{client}
+	}
+
+	gceChecker := &gceReadinessChecker{
+		checker:  readinessChecker,
+		manager:  m,
+		zone:     zone,
+		hostName: host.Name,
+	}
+
+	return WaitForHostReady(gceChecker, 5*time.Minute, 5*time.Second)
+}
+
+func 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 getHostURLWithIns(ins *compute.Instance, config *Config) (*url.URL, error) {
+	addr, err := getHostAddrWithIns(ins)
+	if err != nil {
+		return nil, err
+	}
+	return url.Parse(fmt.Sprintf("%s://%s:%d", config.HostOrchestratorProtocol, addr, config.GCP.HostOrchestratorPort))
+}
+
+func getHostClientWithIns(ins *compute.Instance, config *Config) (HostClient, error) {
+	url, err := getHostURLWithIns(ins, config)
+	if err != nil {
+		return nil, err
+	}
+	return NewNetHostClient(url, config.AllowSelfSignedHostSSLCertificate), nil
 }
 
 func (m *GCEInstanceManager) GetHostClient(zone string, host string) (HostClient, error) {
-	url, err := m.GetHostURL(zone, host)
+	ins, err := m.getHostInstance(zone, host)
 	if err != nil {
 		return nil, err
 	}
-	return NewNetHostClient(url, m.Config.AllowSelfSignedHostSSLCertificate), nil
+	return getHostClientWithIns(ins, &m.Config)
 }
 
 func (m *GCEInstanceManager) getHostInstance(zone string, host string) (*compute.Instance, error) {
@@ -326,6 +412,7 @@ var (
 type opResultGetter struct {
 	Service *compute.Service
 	Op      *compute.Operation
+	Config  *Config
 }
 
 func (g *opResultGetter) Get() (any, error) {

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)