Proposal: Try using Earthly instead of Make for one release#5711
Proposal: Try using Earthly instead of Make for one release#5711negz merged 11 commits intocrossplane:masterfrom
Conversation
50360d8 to
8640ba7
Compare
This comment was marked as outdated.
This comment was marked as outdated.
8640ba7 to
15a6d57
Compare
This comment was marked as outdated.
This comment was marked as outdated.
5048c46 to
469458d
Compare
| - name: Generate Files | ||
| run: earthly --ci +generate | ||
|
|
||
| detect-noop: |
There was a problem hiding this comment.
is this intentionally removing the no-op detection or is that also to be reimplemented?
There was a problem hiding this comment.
I did remove it intentionally. My thinking was that when Earthly has a hot cache it'll handle this automatically. It only rebuilds if the code changes, so changing a Markdown file or similar will be a no-op at the Earthly level.
TBD as to whether that will work though. We'd need to setup remote caching for Earthly to be able to handle it automatically.
There's also a handful of relatively long running CI jobs that don't run in Earthly (yet?) - e.g. fuzz tests and CodeQL. We'd need to either get those running in Earthly, or reintroduce detect-noop. I'd personally prefer to get everything running in Earthly if only to keep things simple and consistent.
turkenh
left a comment
There was a problem hiding this comment.
Thanks @negz for exploring the options here 🙌
I might be one of those who might have invested most to the build submodule, and after some pain and tears, I am feeling relatively comfortable with working with it. However, I can totally see how frustrating it could be for a newcomer. I like the syntax of the Earthfile, especially the fact that it looks like a multi-stage Dockerfile. It look much simpler at the cost of introducing a dependency to an additional tool (different than Make which is available on any system).
I am not sure how it would work when we need to run some docker commands or kind which creates k8s cluster a container since it is already running in a container, but given that you added e2e, I assume it works fine 🤔
Overall, I am fine with experimenting as long as we could have feature parity with the build submodule especially with versioning and artifact publishing / promotion.
42e7be3 to
13603e5
Compare
|
This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation. |
8b47f2d to
0895b79
Compare
@jbw976 I've spent over a week working only on this PR. So I'm fairly confident it's complete, but I can't guarantee I didn't miss anything. There's a lot of history in our Makefiles, and it's pretty tough to grok what things do and why they do it. I don't think we should let perfect be the enemy of good here though - I expect this PR will need followup work to fix up something or other.
@ytsarev There's a |
Signed-off-by: Nic Cope <nicc@rk0n.org>
Signed-off-by: Nic Cope <nicc@rk0n.org>
So that it stops warning about it. Signed-off-by: Nic Cope <nicc@rk0n.org>
The current format uses }, { style, which I find makes it much more
laborious to edit, especially when I want to quickly copy and modify a
block.
Signed-off-by: Nic Cope <nicc@rk0n.org>
Signed-off-by: Nic Cope <nicc@rk0n.org>
b82aa03 to
3093e1e
Compare
My instincts are that "out of the cluster" isn't the important part here - the ability to iterate on build/run rapidly is the important part. That |
We could reimplement this with Earthly but I suspect it's not used. Signed-off-by: Nic Cope <nicc@rk0n.org>
I'm not currently reimplementing the crds.clean target that trims the leading --- from the CRDs because it's unclear whether it's still needed. Signed-off-by: Nic Cope <nicc@rk0n.org>
| # local environment, not a container. The kind cluster will keep running until | ||
| # you run the unhack target. Run hack again to rebuild Crossplane and restart | ||
| # the kind cluster with the new build. | ||
| hack: |
There was a problem hiding this comment.
the hack command ran into an error for me, it looks like unhack was executed and could not execute the kind binary 🤔
❯ earthly +hack
Init 🚀
————————————————————————————————————————————————————————————————————————————————
buildkitd | Found buildkit daemon as docker container (earthly-buildkitd)
Build 🔧
————————————————————————————————————————————————————————————————————————————————
logbus | Setting organization "crossplane" and project "crossplane"
+hack | --> FROM +base
+hack | darwin/arm64
+hack | --> BUILD +unhack
+unhack | --> FROM +base
+kind-setup | --> FROM +base
c/curl:8.8.0 | --> Load metadata curlimages/curl:8.8.0 linux/arm64
Warning: you are not logged into registry-1.docker.io, you may experience rate-limitting when pulling images
+kind-setup | --> FROM curlimages/curl:8.8.0
+kind-setup | [----------] 100% FROM curlimages/curl:8.8.0
+kind-setup | --> RUN curl -fsSLo kind https://github.com/kubernetes-sigs/kind/releases/download/${KIND_VERSION}/kind-${NATIVEOS}-${NATIVEARCH}&&chmod +x kind
+unhack | darwin/arm64
+unhack | --> COPY +kind-setup/kind .hack/kind
+unhack | darwin/arm64
+unhack | --> RUN .hack/kind delete cluster --name crossplane-hack
+unhack | /bin/sh: .hack/kind: cannot execute binary file
+unhack | ERROR Earthfile:95:2
+unhack | The command
+unhack | RUN .hack/kind delete cluster --name crossplane-hack
+unhack | did not complete successfully. Exit code 126
================================== ❌ FAILURE ===================================
+unhack *failed* | Repeating the failure error...
+unhack *failed* | darwin/arm64
+unhack *failed* | --> RUN .hack/kind delete cluster --name crossplane-hack
+unhack *failed* | /bin/sh: .hack/kind: cannot execute binary file
+unhack *failed* | ERROR Earthfile:95:2
+unhack *failed* | The command
+unhack *failed* | RUN .hack/kind delete cluster --name crossplane-hack
+unhack *failed* | did not complete successfully. Exit code 126
+hack | earthfile2llb for +unhack: Earthfile:95:2 apply RUN: unlazy force execution: error calling LocalhostExec: exit code: 126
+hack | in +unhack
🛰️ Reuse cache between CI runs with Earthly Satellites! 2-20X faster than without cache. Generous free tier https://cloud.earthly.dev
There was a problem hiding this comment.
I think the problem was that TARGETOS binaries won't work for LOCALLY targets, since TARGETOS will always be Linux. I've pushed a commit that I hope will fix this. Verified it still works when run from Linux.
This is hacky and I hate it, but I can't think of a better approach. Signed-off-by: Nic Cope <nicc@rk0n.org>
|
|
||
| generate.done: crds.clean crds.patch | ||
|
|
||
| gen-kustomize-crds: |
There was a problem hiding this comment.
I've intentionally skipped reimplementing this target - my hunch is it's not used. We can add it if folks really need it, but I'm not sure how valuable it is.
Its version isn't specified there anymore. Signed-off-by: Nic Cope <nicc@rk0n.org>
Signed-off-by: Nic Cope <nicc@rk0n.org>
For me, the iteration process is incredibly quick. I can run the crossplane core process locally in a terminal tab, receive immediate feedback, make changes, restart the process, and repeat. I can also use advanced local debugging tools like Delve if needed. This method is significantly faster than the |
| | Run linters | ~3 minutes | ~4 minutes | | ||
| | Run unit tests | ~3 minutes | ~2.5 minutes | | ||
| | Publish artifacts | ~12 minutes | ~14 minutes | | ||
| | Run E2E tests | ~12 minutes | ~14 minutes | |
There was a problem hiding this comment.
Curious @negz if you got a sense for any impact on reliability and/or performance of running e2e tests locally. I just thought of this now while running earthly -P +e2e (which I believe runs -test-suite=base). It took about 20 mins to run that suite and it had 10 failures:
DONE 114 tests, 13 skipped, 10 failures in 1196.054s
I'm not sure if that's slower or less reliable - have you developed a sense for that while testing yourself? 🤔
There was a problem hiding this comment.
No idea.
That said, I can't see why it would be slower or less reliable. At the end of the day it's the same code running in a Docker container spun up using kind whether that was done by Make or Earthly.
There was a problem hiding this comment.
FWIW I just ran earthly -P +e2e locally and everything passed. I'm not sure how long it took though - I don't get the DONE summary for whatever reason.
TARGETPLATFORM is incorrect on MacOS, where TARGETOS will be Linux. There we want USERPLATFORM, i.e. Darwin. Signed-off-by: Nic Cope <nicc@rk0n.org>
4eb5645 to
ba9e43f
Compare
Description of your changes
This PR adds a one-pager proposing we adopt https://earthly.dev. Earthly would replace the build submodule.
I propose we try Earthly in
crossplane/crossplaneandcrossplane/crossplane-runtime, for one release. After releasing Crossplane v1.17 we'd assess whether we wanted to commit to Earthly, revert to the build submodule, or something else. I don't think we should attempt to migrate providers until we've lived with Earthly for a while here.The PR also contains a proof-of-concept port of most of this repository's
Makefileto an EarthlyEarthfile. I believe the only thing missing is the rules that publish binaries and Helm charts to S3. Take a look at theEarthfileand the GitHub Actions output to get a sense of what working with Earthly is like.I have:
Runearthly +reviewableto ensure this PR is ready for review.Added or updated unit tests.Added or updated e2e tests.Linked a PR or a docs tracking issue to document this change.Addedbackport release-x.ylabels to auto-backport this PR.Need help with this checklist? See the cheat sheet.