Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

x/build/cmd/tip: deploy instructions for talks.golang.org are non-functional #38120

Closed
dmitshur opened this issue Mar 27, 2020 · 2 comments
Closed
Labels
Builders x/build issues (builders, bots, dashboards) FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@dmitshur
Copy link
Contributor

dmitshur commented Mar 27, 2020

(Factoring this out from #36825 to describe this problem in more detail, without derailing the higher level decision in that issue.)

The deploy instructions for talks.golang.org that are in the x/cmd/tip README have become outdated and cannot be used today.

Doing gcloud --project golang-org app deploy --no-promote talks.yaml in x/build/cmd/tip directory results in:

ERROR: (gcloud.app.deploy) INVALID_ARGUMENT: Legacy health checks are no longer supported for the App Engine Flexible environment. Please remove the 'health_check' section from your app.yaml and configure updated health checks. For instructions on migrating to split health checks see https://cloud.google.com/appengine/docs/flexible/java/migrating-to-split-health-checks

That is easily fixable with:

--- a/cmd/tip/talks.yaml
+++ b/cmd/tip/talks.yaml
@@ -8,5 +8,7 @@ manual_scaling:
 env_variables:
   TIP_BUILDER: 'talks'
 
-health_check:
-  enable_health_check: False
+liveness_check:
+  path: /_ah/health
+readiness_check:
+  path: /_ah/health

The next problem is that the Dockerfile in x/build/cmd/tip directory was modified in CL 176257 with the assumption that it would be called with the x/build root directory as the working directory. That CL modified cmd/tip/Makefile like this:

 docker-image: Dockerfile *.go
-	docker build --force-rm -f Dockerfile --tag=$(IMAGE_PROD):$(VERSION) .
+	docker build --force-rm -f Dockerfile --tag=$(IMAGE_PROD):$(VERSION) ../..

However, that's not compatible with the App Engine-based deploy for talks.golang.org, because doing gcloud app deploy in x/build/cmd/tip causes Cloud Build to try to run Dockerfile with x/build/cmd/tip as the working directory, so statements like COPY go.mod /go/src/golang.org/x/build/go.mod produce errors:

Step #1: Step 6/20 : COPY go.mod /go/src/golang.org/x/build/go.mod
Step #1: COPY failed: stat /var/lib/docker/tmp/docker-builder636121389/go.mod: no such file or directory

/cc @cagedmantis @toothrot @andybons

@dmitshur dmitshur added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Mar 27, 2020
@dmitshur dmitshur added this to the Unreleased milestone Mar 27, 2020
@gopherbot gopherbot added the Builders x/build issues (builders, bots, dashboards) label Mar 27, 2020
@dmitshur dmitshur changed the title x/build/cmd/tip: deploy instructions for talks.golang.org are broken x/build/cmd/tip: deploy instructions for talks.golang.org are non-functional Mar 27, 2020
@dmitshur
Copy link
Contributor Author

dmitshur commented Feb 8, 2021

I revisited and investigated this in some depth last week.

As a reminder, the tip.golang.org website was migrated from App Engine to GKE in CL 37754, but doing the same migration for the talks server was left as a TODO.

That means there are two paths to fix x/build/cmd/tip deployment for talks:

  1. stay with App Engine and fix the paths (without breaking tip.golang.org in the process)
  2. complete the TODO to move talks.golang.org to GKE

I primarily investigated path 2, and did the work of completing the migration to GKE on a staging instance. It worked. However, I found that there's some overhead in updating the DNS entries that would need to be done if we move the talks.golang.org deployment to GKE.

In the end, it seems better to make the decision in #36825 to stick with deployment instructions in x/talks README, and remove the x/build/cmd/tip version for now. If we need to, we can bring it back in the future. Closing because there's nothing more to do here for now.

@dmitshur dmitshur closed this as completed Feb 8, 2021
@gopherbot
Copy link

Change https://golang.org/cl/290529 mentions this issue: cmd/tip: remove ability to serve talks.golang.org

@dmitshur dmitshur self-assigned this Feb 8, 2021
gopherbot pushed a commit to golang/build that referenced this issue Feb 8, 2021
The tip server is primarily about serving tip.golang.org, and it does
today. There was also a time it was used to serve talks.golang.org
with an auto-deploy feature, but that ability has degraded over time
and is currently unused.

After some investigation of our options, we made a decision in issue
golang.org/issue/36825 to remove this broken and unused functionality
in favor of x/talks' own simpler App Engine deployment. If needed,
this can all be brought back and restored as part of future work,
but it's better to delete code and a misleading README until then.

Fixes golang/go#36825.
Updates golang/go#38120.

Change-Id: I5fbca48dea13e871cd04e23b36c8d3acd2f8fef5
Reviewed-on: https://go-review.googlesource.com/c/build/+/290529
Trust: Dmitri Shuralyov <dmitshur@golang.org>
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Alexander Rakoczy <alex@golang.org>
@golang golang locked and limited conversation to collaborators Feb 8, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Builders x/build issues (builders, bots, dashboards) FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

2 participants