Fixes broken local macOS build and completes migration to GH actions #160

codefromthecrypt · 2021-04-03T03:36:03Z

This fixes the broken macOS build and completes migration to GitHub
actions. This doesn't yet fix CI os/x build, as that will still be tracked in
#88.

Most notably, we have no control over process.OpenFiles, so this avoids
testing side-effects of that in Darwin, while mentioning the limitation.
If a future version of go supports that, the test will break (in a good
way!).

On the topic of CI, this moves the unit test and coverage part to GH
actions, and most importantly enables macOS matrix tests. This helps
give confidence for those not using macOS to review change such as
this.

See #88
Fixes #89
Fixes #100

codefromthecrypt · 2021-04-03T03:36:59Z

I use OS/X and verified this works now. It will be very much more productive for me when this is in, because formerly I had to rely on CI to tell if all tests pass or not.

make test
go mod download
go generate ./pkg/...
docker-compose up -d
getenvoy_registry_1 is up-to-date
go test   ./pkg/...
?   	github.com/tetratelabs/getenvoy/pkg/binary	[no test files]
ok  	github.com/tetratelabs/getenvoy/pkg/binary/envoy	0.791s
ok  	github.com/tetratelabs/getenvoy/pkg/binary/envoy/controlplane	8.704s
ok  	github.com/tetratelabs/getenvoy/pkg/binary/envoy/debug	10.491s
?   	github.com/tetratelabs/getenvoy/pkg/binary/envoytest	[no test files]
ok  	github.com/tetratelabs/getenvoy/pkg/cmd	1.996s
ok  	github.com/tetratelabs/getenvoy/pkg/cmd/extension	3.009s
ok  	github.com/tetratelabs/getenvoy/pkg/cmd/extension/build	0.687s
ok  	github.com/tetratelabs/getenvoy/pkg/cmd/extension/clean	1.693s
?   	github.com/tetratelabs/getenvoy/pkg/cmd/extension/common	[no test files]
ok  	github.com/tetratelabs/getenvoy/pkg/cmd/extension/example	2.669s
?   	github.com/tetratelabs/getenvoy/pkg/cmd/extension/globals	[no test files]
ok  	github.com/tetratelabs/getenvoy/pkg/cmd/extension/init	2.327s
ok  	github.com/tetratelabs/getenvoy/pkg/cmd/extension/push	1.039s
ok  	github.com/tetratelabs/getenvoy/pkg/cmd/extension/run	7.637s
ok  	github.com/tetratelabs/getenvoy/pkg/cmd/extension/test	1.365s
?   	github.com/tetratelabs/getenvoy/pkg/common	[no test files]
?   	github.com/tetratelabs/getenvoy/pkg/errors	[no test files]
?   	github.com/tetratelabs/getenvoy/pkg/extension/example/init/registry	[no test files]
?   	github.com/tetratelabs/getenvoy/pkg/extension/example/init/registry/templates	[no test files]
ok  	github.com/tetratelabs/getenvoy/pkg/extension/init	0.371s
?   	github.com/tetratelabs/getenvoy/pkg/extension/init/templates	[no test files]
?   	github.com/tetratelabs/getenvoy/pkg/extension/manager	[no test files]
?   	github.com/tetratelabs/getenvoy/pkg/extension/wasmimage	[no test files]
?   	github.com/tetratelabs/getenvoy/pkg/extension/workspace	[no test files]
?   	github.com/tetratelabs/getenvoy/pkg/extension/workspace/config	[no test files]
?   	github.com/tetratelabs/getenvoy/pkg/extension/workspace/config/example	[no test files]
ok  	github.com/tetratelabs/getenvoy/pkg/extension/workspace/config/extension	0.227s
ok  	github.com/tetratelabs/getenvoy/pkg/extension/workspace/config/toolchain/builtin	0.477s
?   	github.com/tetratelabs/getenvoy/pkg/extension/workspace/example	[no test files]
ok  	github.com/tetratelabs/getenvoy/pkg/extension/workspace/example/envoy/template	0.313s
ok  	github.com/tetratelabs/getenvoy/pkg/extension/workspace/example/envoy/util	0.494s
?   	github.com/tetratelabs/getenvoy/pkg/extension/workspace/example/runtime	[no test files]
ok  	github.com/tetratelabs/getenvoy/pkg/extension/workspace/example/runtime/configdir	0.900s
ok  	github.com/tetratelabs/getenvoy/pkg/extension/workspace/example/runtime/getenvoy	4.389s
?   	github.com/tetratelabs/getenvoy/pkg/extension/workspace/fs	[no test files]
?   	github.com/tetratelabs/getenvoy/pkg/extension/workspace/model	[no test files]
ok  	github.com/tetratelabs/getenvoy/pkg/extension/workspace/toolchain	0.582s
ok  	github.com/tetratelabs/getenvoy/pkg/extension/workspace/toolchain/builtin	0.433s
?   	github.com/tetratelabs/getenvoy/pkg/extension/workspace/toolchain/registry	[no test files]
?   	github.com/tetratelabs/getenvoy/pkg/extension/workspace/toolchain/types	[no test files]
ok  	github.com/tetratelabs/getenvoy/pkg/flavors	0.303s
ok  	github.com/tetratelabs/getenvoy/pkg/flavors/postgres	0.196s
ok  	github.com/tetratelabs/getenvoy/pkg/manifest	0.347s
ok  	github.com/tetratelabs/getenvoy/pkg/pii	0.189s
?   	github.com/tetratelabs/getenvoy/pkg/test/cmd	[no test files]
?   	github.com/tetratelabs/getenvoy/pkg/test/manifest	[no test files]
ok  	github.com/tetratelabs/getenvoy/pkg/transport	0.274s
ok  	github.com/tetratelabs/getenvoy/pkg/types	0.221s
ok  	github.com/tetratelabs/getenvoy/pkg/util/args	0.284s
ok  	github.com/tetratelabs/getenvoy/pkg/util/cmd	0.261s
?   	github.com/tetratelabs/getenvoy/pkg/util/cmd/errors	[no test files]
?   	github.com/tetratelabs/getenvoy/pkg/util/docker	[no test files]
ok  	github.com/tetratelabs/getenvoy/pkg/util/exec	3.437s
?   	github.com/tetratelabs/getenvoy/pkg/util/io	[no test files]
ok  	github.com/tetratelabs/getenvoy/pkg/util/os	1.298s
?   	github.com/tetratelabs/getenvoy/pkg/util/scaffold	[no test files]
?   	github.com/tetratelabs/getenvoy/pkg/util/ui	[no test files]
ok  	github.com/tetratelabs/getenvoy/pkg/version	2.803s

codefromthecrypt · 2021-04-03T03:37:45Z

pkg/binary/envoy/controlplane/istio_test.go

@@ -32,42 +31,44 @@ import (
 	"github.com/tetratelabs/getenvoy/pkg/binary/envoytest"
 )

-func TestMain(m *testing.M) {


I moved this into the test preconditions as it isn't necessary to do this for anything in the package

codefromthecrypt · 2021-04-03T03:38:30Z

pkg/binary/envoy/controlplane/istio_test.go

+		// In a normal macOS setup, you cannot write to /dev/stdout, which is the default path here.
+		// While not Docker-specific, there are related notes here https://github.com/moby/moby/issues/31243
+		// Since this test doesn't read access logs anyway, the easier workaround is to disable access logging.
+		args.MeshConfig.AccessLogFile = ""


Key fix for darwin is here, and when there's a better solution (than running as root or changing host perms), hopefully this comment explains enough.

codefromthecrypt · 2021-04-03T03:38:55Z

pkg/binary/envoy/debug/lsof_test.go

+	require.NoError(t, err, "error stating %v", path)
+
+	if runtime.GOOS == `darwin` { // process.OpenFiles in unsupported, so this feature won't work
+		require.Empty(t, f.Size(), "file %v was not empty", path)


better to do a negative test than always fail osx builds

codefromthecrypt · 2021-04-03T06:56:28Z

circleci needs to be removed as a required check to merge this (we no longer use it)

codefromthecrypt · 2021-04-03T07:02:53Z

.github/workflows/commit.yaml

+    strategy:
+      matrix:
+        runner:
+          - os: macos-latest


in circleci, we only tested ubuntu. this also tests macos which saves devs like me who use it. if this fails, we can assume dev workflow is busted.

codefromthecrypt · 2021-04-03T07:46:55Z

E2E failures are unrelated as this doesn't change anything E2E related. It appears we hit docker.io rate limits again #149

mathetake · 2021-04-05T09:18:09Z

@codefromthecrypt The macos test has never succeeded, and I must see that pass before merging.

mathetake · 2021-04-05T09:19:32Z

looks like it's stuck somewhere in the tests.

mathetake · 2021-04-05T13:02:10Z

I reproduced the same hang of test in my local mac (with the latest docker for mac on Catalina 10.15.7).

codefromthecrypt · 2021-04-05T23:08:45Z

I have a hunch the hang is timeout related, our "unit tests" perform I/O, download envoy etc, it seems easy to get over 60s, especially in osx. I'll remove that for a job timeout especially until test migrations occur.

codefromthecrypt · 2021-04-06T06:18:40Z

even though the macos build is fixed for me locally, I think it isn't quite working in CI. I'll un-tag this from fixing OS/X even if it is fine local. At any rate, this is progress from before as before even local didn't work. Once we migrate completely off ginkgo, it should be easier to identify what's wrong as timeouts will be coherent then.

codefromthecrypt · 2021-04-06T08:22:32Z

not sure why GH actions fell of the checks, but this is the run https://github.com/tetratelabs/getenvoy/actions/runs/721808158

Most notably, we have no control over process.OpenFiles, so this avoids testing side-effects of that in Darwin, while mentioning the limitation. If a future version of go supports that, the test will break (in a good way!). Signed-off-by: Adrian Cole <adrian@tetrate.io>

Signed-off-by: Adrian Cole <adrian@tetrate.io>

codefromthecrypt · 2021-04-06T09:34:47Z

@mathetake fyi there might be some cleanup in circleci so that builds aren't attempted anymore on any branch

codefromthecrypt requested a review from mathetake April 3, 2021 03:37

codefromthecrypt commented Apr 3, 2021

View reviewed changes

codefromthecrypt added the tech debt label Apr 3, 2021

codefromthecrypt force-pushed the fix-osx branch 2 times, most recently from 37e7961 to 49f81f9 Compare April 3, 2021 06:55

codefromthecrypt changed the title ~~Fixes broken macOS build~~ Fixes broken macOS build and completes migration to GH actions Apr 3, 2021

codefromthecrypt commented Apr 3, 2021

View reviewed changes

codefromthecrypt mentioned this pull request Apr 3, 2021

WIP: updates to latest dependencies #128

Closed

mathetake approved these changes Apr 5, 2021

View reviewed changes

codefromthecrypt force-pushed the fix-osx branch from a10e3ee to 06e6e8e Compare April 5, 2021 23:05

codefromthecrypt changed the title ~~Fixes broken macOS build and completes migration to GH actions~~ Fixes broken local macOS build and completes migration to GH actions Apr 6, 2021

codefromthecrypt force-pushed the fix-osx branch from 6805284 to a7144fe Compare April 6, 2021 07:56

Adrian Cole added 5 commits April 6, 2021 16:24

Adds verbose flag

13b2094

Signed-off-by: Adrian Cole <adrian@tetrate.io>

removes early timeout to keep things simple.

205c645

Signed-off-by: Adrian Cole <adrian@tetrate.io>

inline fetch of envoy

5830704

temporarily disables CI build of os/x

894ff3e

Signed-off-by: Adrian Cole <adrian@tetrate.io>

codefromthecrypt force-pushed the fix-osx branch from a7144fe to 894ff3e Compare April 6, 2021 08:26

codefromthecrypt merged commit 21c4632 into master Apr 6, 2021

codefromthecrypt deleted the fix-osx branch April 6, 2021 09:33

mathetake mentioned this pull request Apr 6, 2021

Remove CircleCI badge #165

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes broken local macOS build and completes migration to GH actions #160

Fixes broken local macOS build and completes migration to GH actions #160

codefromthecrypt commented Apr 3, 2021 •

edited

Loading

codefromthecrypt commented Apr 3, 2021

codefromthecrypt Apr 3, 2021

codefromthecrypt Apr 3, 2021

codefromthecrypt Apr 3, 2021

codefromthecrypt commented Apr 3, 2021

codefromthecrypt Apr 3, 2021

codefromthecrypt commented Apr 3, 2021

mathetake commented Apr 5, 2021

mathetake commented Apr 5, 2021

mathetake commented Apr 5, 2021

codefromthecrypt commented Apr 5, 2021

codefromthecrypt commented Apr 6, 2021

codefromthecrypt commented Apr 6, 2021

codefromthecrypt commented Apr 6, 2021

Fixes broken local macOS build and completes migration to GH actions #160

Fixes broken local macOS build and completes migration to GH actions #160

Conversation

codefromthecrypt commented Apr 3, 2021 • edited Loading

codefromthecrypt commented Apr 3, 2021

codefromthecrypt Apr 3, 2021

Choose a reason for hiding this comment

codefromthecrypt Apr 3, 2021

Choose a reason for hiding this comment

codefromthecrypt Apr 3, 2021

Choose a reason for hiding this comment

codefromthecrypt commented Apr 3, 2021

codefromthecrypt Apr 3, 2021

Choose a reason for hiding this comment

codefromthecrypt commented Apr 3, 2021

mathetake commented Apr 5, 2021

mathetake commented Apr 5, 2021

mathetake commented Apr 5, 2021

codefromthecrypt commented Apr 5, 2021

codefromthecrypt commented Apr 6, 2021

codefromthecrypt commented Apr 6, 2021

codefromthecrypt commented Apr 6, 2021

codefromthecrypt commented Apr 3, 2021 •

edited

Loading