Skip to content

Commit

Permalink
Improve FAH create & delete (#6726)
Browse files Browse the repository at this point in the history
* Improve FAH create & delete

1. Create now sets deployment-tool so we can track where users are
   coming from
2. Create now sets a rollout policy like the Console
3. Delete now works
4. Delete no longer makes buildId required

* Fix test breakage where labels were wrong

* Remove unused stub

* Revert unwanted changes
  • Loading branch information
inlined committed Feb 5, 2024
1 parent d97d13c commit 1473c04
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 16 deletions.
31 changes: 24 additions & 7 deletions src/gcp/apphosting.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ export type BuildOutputOnlyFields =
| "error"
| "image"
| "sourceRef"
| "buildLogUri"
| "buildLogsUri"
| "reconciling"
| "uuid"
| "etag"
Expand Down Expand Up @@ -197,10 +197,20 @@ export interface RolloutPolicy {
// end oneof trigger
stages?: RolloutStage[];
disabled?: boolean;

// TODO: This will be undefined if disabled is not true, right?
disabledTime: string;
}

export type RolloutPolicyOutputOnlyFields = "disabledtime";
export type RolloutPolicyOutputOnlyFields = "disabledTime";

export type RolloutPolicyInput = Omit<RolloutPolicy, RolloutPolicyOutputOnlyFields | "stages"> & {
stages: Omit<RolloutStage, "startTime" | "endTime">[];
};

export type TrafficInput = Omit<Traffic, TrafficOutputOnlyFields | "rolloutPolicy"> & {
rolloutPolicy: RolloutPolicyInput;
};

export type RolloutProgression =
| "PROGRESSION_UNSPECIFIED"
Expand Down Expand Up @@ -304,8 +314,8 @@ export async function deleteBackend(
location: string,
backendId: string,
): Promise<Operation> {
const name = `projects/${projectId}/locations/${location}/backends/${backendId}?force=true`;
const res = await client.delete<Operation>(name);
const name = `projects/${projectId}/locations/${location}/backends/${backendId}`;
const res = await client.delete<Operation>(name, { queryParams: { force: "true" } });

return res.body;
}
Expand Down Expand Up @@ -393,14 +403,21 @@ export async function updateTraffic(
projectId: string,
location: string,
backendId: string,
traffic: Omit<Traffic, TrafficOutputOnlyFields | "name">,
traffic: Omit<TrafficInput, "name">,
): Promise<Operation> {
const fieldMasks = proto.fieldMasks(traffic);
// BUG(b/322891558): setting deep fields on rolloutPolicy doesn't work for some
// reason. Create a copy without deep fields to force the updateMask to be
// correct.
const trafficCopy = { ...traffic };
if ("rolloutPolicy" in traffic) {
trafficCopy.rolloutPolicy = {} as any;
}
const fieldMasks = proto.fieldMasks(trafficCopy);
const queryParams = {
updateMask: fieldMasks.join(","),
};
const name = `projects/${projectId}/locations/${location}/backends/${backendId}/traffic`;
const res = await client.patch<Omit<Traffic, TrafficOutputOnlyFields>, Operation>(
const res = await client.patch<TrafficInput, Operation>(
name,
{ ...traffic, name },
{
Expand Down
43 changes: 35 additions & 8 deletions src/init/features/apphosting/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import { FirebaseError } from "../../../error";
import { promptOnce } from "../../../prompt";
import { DEFAULT_REGION } from "./constants";
import { ensure } from "../../../ensureApiEnabled";
import * as deploymentTool from "../../../deploymentTool";

const apphostingPollerOptions: Omit<poller.OperationPollerOptions, "operationResourceName"> = {
apiOrigin: apphostingOrigin,
Expand Down Expand Up @@ -76,6 +77,33 @@ export async function doSetup(setup: any, projectId: string): Promise<void> {

const backend = await onboardBackend(projectId, location, backendId);

// TODO: Once tag patterns are implemented, prompt which method the user
// prefers. We could reduce the nubmer of questions asked by letting people
// enter tag:<pattern>?
const branch = await promptOnce({
name: "branch",
type: "input",
default: "main",
message: "Pick a branch for continuous deployment",
});
const traffic: Omit<apphosting.TrafficInput, "name"> = {
rolloutPolicy: {
codebaseBranch: branch,
stages: [
{
progression: "IMMEDIATE",
targetPercent: 100,
},
],
},
};
const op = await apphosting.updateTraffic(projectId, location, backendId, traffic);
await poller.pollOperation<apphosting.Traffic>({
...apphostingPollerOptions,
pollerName: `updateTraffic-${projectId}-${location}-${backendId}`,
operationResourceName: op.name,
});

const confirmRollout = await promptOnce({
type: "confirm",
name: "rollout",
Expand All @@ -89,13 +117,6 @@ export async function doSetup(setup: any, projectId: string): Promise<void> {
return;
}

const branch = await promptOnce({
name: "branch",
type: "input",
default: "main",
message: "Which branch do you want to deploy?",
});

const { build } = await onboardRollout(projectId, location, backendId, {
source: {
codebase: {
Expand All @@ -105,6 +126,12 @@ export async function doSetup(setup: any, projectId: string): Promise<void> {
});

if (build.state !== "READY") {
if (!build.buildLogsUri) {
throw new FirebaseError(
"Failed to build your app, but failed to get build logs as well. " +
"This is an internal error and should be reported",
);
}
throw new FirebaseError(
`Failed to build your app. Please inspect the build logs at ${build.buildLogsUri}.`,
{ children: [build.error] },
Expand Down Expand Up @@ -137,7 +164,7 @@ function toBackend(cloudBuildConnRepo: Repository): Omit<Backend, BackendOutputO
repository: `${cloudBuildConnRepo.name}`,
rootDirectory: "/",
},
labels: {},
labels: deploymentTool.labels(),
};
}

Expand Down
3 changes: 2 additions & 1 deletion src/test/init/apphosting/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import * as poller from "../../../operation-poller";
import * as prompt from "../../../prompt";
import { createBackend, onboardBackend } from "../../../init/features/apphosting/index";
import { FirebaseError } from "../../../error";
import * as deploymentTool from "../../../deploymentTool";

describe("operationsConverter", () => {
const sandbox: sinon.SinonSandbox = sinon.createSandbox();
Expand Down Expand Up @@ -66,7 +67,7 @@ describe("operationsConverter", () => {
repository: cloudBuildConnRepo.name,
rootDirectory: "/",
},
labels: {},
labels: deploymentTool.labels(),
};

it("should createBackend", async () => {
Expand Down

0 comments on commit 1473c04

Please sign in to comment.