#CI refactor

1 messages · Page 1 of 1 (latest)

brisk raptor
#

Tentative plan (can be parallelize)

  • github.com/dagger/dagger/version
  • github.com/dagger/dagger/modules/go
  • github.com/dagger/dagger/modules/supermod
  • github.com/dagger/dagger/modules/superdir (might as well rename dirdiff while we're at it)
  • github.com/dagger/dagger/go/sdk

Then:

  • github.com/dagger/dagger/engine (bigger chunk)
#

Let's coordinate here

#

I'm taking version

#

FYI @summer root @shell portal

brisk raptor
#

Got sidetracked by in-person meetings, but wrapping up my day with a first draft of the version PR

somber dust
#

Left some comments on the split-out PR, will try and find some time today and make the changes I want myself (and push them as a separate commit, so we can easily revert)

#

But left as comments just in case I can't find the time!

brisk raptor
#

Notes from check-in call:

  • Participants: @gentle orchid, @somber dust, @brisk raptor
  • Version PR: overall looks good. Failing xxx-dev checks in CI, because the dev version string is not the same, and the format matters.
  • Problem: no way to differentiate clean from dirty checkout (necessary for -dev)
  • Solution: partial git diff using the git-dir and inputs directories we already have. yay!
  • @gentle orchid and @brisk raptor will pair on the diff implementation
  • @somber dust will take over tomorrow and make the dev format identical (for go tooling interop)
brisk raptor
#

@gentle orchid quick 5mn break then ready to pair 👍

brisk raptor
#

joined dev-audio, ready when you are

brisk raptor
#

@somber dust we fixed dev detection logic 🙂 You can now call Version.dev() and it will correctly tell you if any of the input files have changed

somber dust
#

nice

#

picking this up now 🙂

#

😢 working on this module is not wonderfully fun - it takes me about 30s just to compute a version identifier

somber dust
#

okay, almost there 🙂

#

just working on something with the dev detection - it currently doesn't work with newly added files, so trying an experiment to see if i can get that working

#

the git cloned repository doesn't have any tags in it

#

i think we can update that for our clone - but the issue is, it won't work well for the entirely remote pattern where we do -m github.com/dagger/dagger - because that's a shallow clone (and so won't have the tags)

#

😢 ahh it does work if you specify the tag in the git url

#

ahhhh i worked out how to entirely remove the weird .changes/ markdown file analysis, and just git tags, but because we don't have access to all of them, i can't

regal hearth
#

@somber dust maybe you could just find all the tags with git ls-remote?

#

there's a trick for that iirc

somber dust
#

unfortunately, one issue with this - now we need to have ssh auth sock in the module

somber dust
#

oh well, we can just hardcode the upstream url 😛 sucks, but better than pulling from "origin", which is potentially wrong (e.g. for me, this is jedevc/dagger, which has no tags, and sometimes has a collection of absolutely wild test tags)

regal hearth
#

that's a good point - it should probably be based on the "source of truth" for dagger anyway

somber dust
#

super sad thing i just discovered - if you do llb.Git using a commit sha that's not on the default branch... you can't

#

which means you can't dag.Git with a commit that's not on the default branch

#

gonna try and fix that upstream, since there's no good reason for that limitation

#

it sucks, cause it means you can't do -m github.com/dagger/dagger@<commit sha> you have to do -m github.com/dagger/dagger@refs/... (and really hope there's a ref that points to the thing you want)

#

e.g. imagine hell if you push while a ci job is running 😄 and you get the new content instead of the old one

brisk raptor
#

(joining another call as you know, sorry)

somber dust
gentle orchid
somber dust
#

yeah, i think we need an "unshallow" option for cases like this

#

or similar

#

technically, all i want is certain branches to be unshallowed

#

(and all the tags)

somber dust
#

okay, writing up for the day and passing off to @brisk raptor @gentle orchid:

  • i'm happy with the current state of the module, i got it working to the previous version formats
  • i also added some very small improvements since we have all this rich git data now - i killed the changelog .md parsing to get the latest release, and now we get this from git (goodbye fragile changelogs, it's my gift to you @brisk raptor). i also did something clever, and we set better engine image tags in the cli when building from a branch that's not main
  • tidied and refactored around, and got everything to use the same wolfi base (to avoid extra overhead)
  • changed how the Dev functions works and renamed it to Dirty - it now operates exactly like git status (so handled added files or staged changed, which git diff doesn't), i grabbed the logic from similar in buildx (where i worked on this problem before)

I really like this btw:

$ dagger call -m github.com/dagger/dagger/version@pull/8564/head  --git-dir=https://github.com/dagger/dagger#pull/8564/head

_type: Version
imageTag: 72a848aa27ceca12609a12821d6c3732b62df9d7
lastReleaseVersion: v0.13.3
nextReleaseVersion: v0.13.4
version: v0.13.4-240926174249-f36402ee4e22

At a glance, you've got the version, the image tag used by it, and finally the last/next version

#

I think it's done? Just needs reviews, but if it could avoid being merged until tomorrow, I'd like another glance through it with fresh eyes 😄

brisk raptor
#

Nice, thanks!

brisk raptor
#

@somber dust can you explain how the distconsts part works?

somber dust
#

not related to the current version pr right?

brisk raptor
#

yes I mean specifically this PR's use of distconsts

#

I'm familiar with our existing use of distconsts in general

somber dust
#

? it doesn't use distconsts

brisk raptor
#

oh sorry. engine/consts

somber dust
#

sorry, getting a little distracted here too lol

brisk raptor
#

Oh you literally just changed the comment

#

nevermind

#

all good 🙂

#

So should I merge it?

somber dust
#

if it looks good to you, approve it 😄
if there is a rush, feel free to merge, but i'd like to take another glance at it tomorrow morning when i've had coffee to make sure that the logic actually pans out - also if it breaks on merge, i'd like to be around to help fix, and i'm just about to sign off and pack for san diego 😛

brisk raptor
#

@somber dust are you sure you didn't accidentally remove the un-shallowing in your refactor?

somber dust
#

and added more 😢

#

it sucks so so so much, but i have some ideas for how to get it out in the future (and doing dagger builds offline isn't really a thing anyways, so not really the most urgent thing)

#

contextual git pleading_lemon pleading_lemon (i think i will make this my hackathon topic)

brisk raptor
#

So if I understand correctly, a downside of our current approach is that, even if we don't need to fetch more from upstream, because we're working from a local checkout, we will still execute those commands and they will require talking to upstream, even though they will end up downloading nothing new ?

#

But at least, no wasted downloads - I'm guessing if there's nothing to fetch, it's just a wasted roundtrip

somber dust
#

yup 😢
buttt - one cool thing is it means that if someone tagged a release, it does mean you still get the right result, even if you haven't re-pulled (on your local repo)

somber dust
brisk raptor
#

Is there a potential cache invalidation problem with these commands?

#

I guess not (except for the obscure case where you pulled commits just before they were tagged)

somber dust
#

yeah, it's a realllllly obscure case

brisk raptor
#

so in the 99.99% case: new commits upstream, so new git dir, so new fetch command, so you get fresh tags, and your version is up to date

somber dust
#

exactly

brisk raptor
#

but in between git pulls: proper caching of those fetches

#

not too bad!

#

and the code looks readable and clean

somber dust
#

i really like the way you made the new VersionTag struct, i think it's a very neat way to think about this

#

because now you just go "git give me every release for a component in this repo", and by having it sorted, it's really easy to determine which the latest is

#

merged 👀 👀
party_blob party_blob party_blob

brisk raptor
#

Right it kind of just fell into place:

  • "Argh I wish I had this information"
  • "Oh wait, I could easily fetch it"
  • "Ah the data structure just writes itself!"
somber dust
#

first win as the little ci task force feels good 😄

brisk raptor
#

Yes definitely. Thank you for the help.

somber dust
#

lol, and it immediately turned red on main

brisk raptor
#

@shell portal @summer root we're definitely pulling you into the next one 😛

brisk raptor
somber dust
#

gr

#

i know what it is

brisk raptor
#

Something different about main?

somber dust
#

tl;dr - git fetch by default does not want to update a ref that you have checked out

#

hm, actually there's a reason that flag tells you not to use it lol

#

lemme push a different fix

somber dust
#

merged that but now found even more things sorry

#

git repos are a nightmare

#

shallow git repositories are so weird to work with (and turns out technically i've been working with a shallow repo for the last year lol)

somber dust
#

i'm heading off to go pack now 👋

brisk raptor
#

Thank you!

brisk raptor
#

Got a few problems not fixed by the PR:

#
  1. New error replaced the .git not found error
$ call -m github.com/dagger/dagger/version@pull/8589/head
invoke: no releases found
#
  1. Execution of the "unshallow" is considerably slower than before, do we really need to fetch this much?
#
  1. Version string for a stable release tag no longer produces a clean stable version
$ dagger call -m github.com/dagger/dagger/version@pull/8589/head --git-dir=https://github.com/dagger/dagger#v0.13.2
v0.13.4-010101000000-dev-ac5c7f4a3265

(same problem in main)

$ dagger call -m github.com/dagger/dagger/version --git-dir=https://github.com/dagger/dagger#v0.13.2
v0.13.4-010101000000-dev-ac5c7f4a3265
#

It's so slow

somber dust
somber dust
somber dust
somber dust
#

essentially you've got the git directory from the release, but the contents of pr 8589

brisk raptor
#

oh right - I need to override inputs also

somber dust
#

looking into a quick optimization right now, and also worked out a clever way to remove the .changes input

#

sigh contextual git is gonna be such a lifesaver

#

this is such a nightmare without it

#

while sitting around in the airport, if i have some acceptable wifi, i might try writing some tests for this module

#

i'd like to be confident that it works

#

i've managed to shave off a few seconds of fetch by cramming all the git fetch into one single command

somber dust
#

okay pushed some changes, all in separate commits (but should be squashed when merged)

  • i've fixed the case where no git metadata is available
  • i've added some optimizations to group all the git fetches together
  • i've done a cool trick where we can actually load the .changes/.nextfile from git, which is actually way more accurate for some edge cases, but we still need to hold onto the old way for when no git metadata is around (v0.13.4 will have that fix, so i've added a note we can remove it)
brisk raptor
somber dust
brisk raptor
#

I was only using it to determine if a commit was tagged

brisk raptor
#

@somber dust is that fix PR ready to merge?

#

I almost did, but saw a failed check. Seems to be gone now

somber dust
#

I would hold off, since I'd like to be around to fix it if it breaks again 😔

brisk raptor
#

OK but this is killing me:

$ dagger call -m github.com/dagger/dagger dev terminal
✔ connect 0.3s
✔ initialize 3.2s
✔ prepare 0.0s
✘ daggerDev: DaggerDev! 5.6s
! call constructor: process "/runtime" did not complete successfully: exit code: 2
┃ invoke: input: version.version resolve: call function "Version": process "/runtime" did not complete successfully: exit code: 2
┃
┃ Stdout:
┃ invoke: input: container.import.withMountedTemp.withMountedTemp.withWorkdir.withWorkdir.withDirectory.withDirectory.withExec.withExe
┃ withExec.withExec.directory resolve: process "sh -c git checkout $(git rev-parse HEAD)" did not complete successfully: exit code: 12
┃
┃ Stderr:
┃ fatal: not a git repository (or any of the parent directories): .git
┃ fatal: not a git repository (or any of the parent directories): .git
  ✔ version: Version! 0.1s
  ✘ Version.version: String! 5.3s
  ! call function "Version": process "/runtime" did not complete successfully: exit code: 2
  ┃ invoke: input: container.import.withMountedTemp.withMountedTemp.withWorkdir.withWorkdir.withDirectory.withDirectory.withExec.withE
  ┃ withExec.withExec.directory resolve: process "sh -c git checkout $(git rev-parse HEAD)" did not complete successfully: exit code:
  ┃
  ┃ Stderr:
  ┃ fatal: not a git repository (or any of the parent directories): .git
  ┃ fatal: not a git repository (or any of the parent directories): .git
    ✔ Wolfi.container(packages: ["git"]): Container! 5.2s
    ✘ Container.directory(path: "."): Directory! 0.1s
    ! process "sh -c git checkout $(git rev-parse HEAD)" did not complete successfully: exit code: 128

Full trace at https://dagger.cloud/dagger/traces/78d52f6eee06c50a7887f0bd36059652

Error: response from query: input: daggerDev resolve: call constructor: process "/runtime" did not complete successfully: exit code: 2

Stdout:
invoke: input: version.version resolve: call function "Version": process "/runtime" did not complete successfully: exit code: 2

Stdout:
invoke: input: container.import.withMountedTemp.withMountedTemp.withWorkdir.withWorkdir.withDirectory.withDirectory.withExec.withExec.withExec.withExec.directory resolve: process "sh -c git checkout $(git rev-parse HEAD)" did not complete successfully: exit code: 128

Stderr:
fatal: not a git repository (or any of the parent directories): .git
fatal: not a git repository (or any of the parent directories): .git
Run 'dagger call dev terminal --help' for usage.
#

It's bad luck, but literally 5 times a day I point to this example as the proof of how wonderful and unique Dagger is

#

and now it's broken...

somber dust
#

Sure go for it - but if it breaks, you're on deck to fix it

brisk raptor
#

lol fair enough

#

I'll be on deck to reverse the PR immediately 😛

#

@somber dust other than possible risk of something going wrong, is there any known issue with the PR at this point?

somber dust
#

Nope no known issues

#

Should hopefully be fine 😂

brisk raptor
#

OK then

somber dust
brisk raptor
#

It's working!

#

(so far)

somber dust
#

Fyi, I think we normally do merges as squash-and-merge, instead of rebate-and-merge (even though rebase-and-merge is my personal preference 😂)

brisk raptor
#

Ha ha, I just clicked on that without thinking about it, since a) it's obviously the right way to do it IMO and b) it's usually disabled so I never have to think about it

#

I guess it was recently enabled?

somber dust
#

Segfault in the compiler?

brisk raptor
#

Why is it trying to publish?

somber dust
#

Immmmm gonna say that's not our fault

somber dust
brisk raptor
#

oh right

somber dust
#

Not tagged, but they still go to the registry and clj

brisk raptor
#

re-running

#

everything else seems to be passing

somber dust
#

Yeahhh that job also runs as test publish I think, so we would have tried to build it in the PR

#

If it properly breaks again, weiiiird

#

Anyways! On that note, I'm off to bed lol, big travel day for me tomorrow

brisk raptor
#

Safe travels, see you on the other side!

brisk raptor
#

@somber dust are you able to join?

somber dust
#

eek yes

#

one moment

somber dust
#

assuming we're skipping today? given there's fastly at the same time?

#

one question for offline - what's the plan to move forwards with the gha module?
are we holding that work in light of native ci?

if we're going ahead with it, can we import it into modules/ soon? i'd like to have it all live in the same place if we can (i was looking at making some fixes/changes, and it's fiddly to work on across multiple repos)

#

also if we're going ahead, i was gonna try and simplify to remove the complex dev engine bits, remove the last bits of mage, and convert everything over to use it

#

the reasoning is - our ci config is now split: this makes changes hard.
i think we either need to go backwards (to what we had before), or go forwards (and get everything consistent) - i have a massive preference for forwards 😄

brisk raptor
#

can't make the call today sorry conflict

somber dust
#

good news! the version module is working correctly on tags 🎉

#

just tagged the release v0.13.4, expected pain and suffering, all just works party_blob

brisk raptor
#

I can open the PR to move gha into modules

#

re: dev engine. I did think "mm this should print be dagger-in-dagger with a regular gha config" wdyt?

somber dust
#

awesome 🎉 i'll plan to spend a decent amount of time on thursday/friday doing some work here 🙂

somber dust
#

the reason it's hard, is then this case becomes triple-nested dagger

somber dust
#

Noticed the recurring invite is still on? I can't make it today, just gone out for a haircut 💈

#

Me and @matipan have mostly finished up in https://github.com/dagger/dagger/pull/8715, would appreciate sync/async comments - the idea is to start with this import + feature, then to keep iterating on it in main so we keep up velocity on this (it's an important part of one-click releasing now)

brisk raptor
#

@somber dust yeah the call is canceled. Will remove the invite now. Didn't have time yesterday since it was so busy

brisk raptor
#

@shell portal @somber dust I'm still reviewing that GHA pr, sorry for being slow - it's surprisingly difficult to review the API of a module in a PR, I got used to just having the daggerverse API docs page..

#

Note, @summer root @finite lava this 👆 might be a good use case for your "open in Cloud" button idea. I would love to click a button in that PR right now, and see the module page in Cloud, with the docs, an interactive shell to play around and everything

brisk raptor
#

Update: this has been de-prioritized, which makes me sad. I am going to try carrying it on my own as a side project. I think it's a waste that our own CI is something we're slightly ashamed of, and don't proudly point our users to it as a golden example. I hope we can free up engineering cycles to do that soon.

#

So far we got one piece merged: the version submodule. I will try to find another small win to regain momentum.

gentle orchid
brisk raptor
#

Next up: modules/go

#

Then I will try cmd/dagger which depends on go and version

#

Then, an easy one: helm which depends on cmd/dagger

#

So much good stuff rotting in that big PR... makes me sad

brisk raptor
brisk raptor
brisk raptor
#

If you have any cycles to review, I'd appreciate it 🙏

#

those tend to catch conflicts quickly

somber dust
#

(left some coffee shop comments, since I wanted a chance to weigh in!)
Some concerns about the overlays, otherwise, I'm a fan of the direction generally - will help out some more next week ❤️

#

Hopefully this aligns with release improvements enough that I can steal some time away to work on things here

brisk raptor
#

hey aren't you on vacation?

#

yeah the overlays are an experiment. Cool use of interfaces IMO, and directionally correct, but lots of room for improvement. Some of it is really because interfaces themselves are rough...

#

either way thank you!

somber dust
# brisk raptor hey aren't you on vacation?

Yeaaa the weather here in Denver is dreadful though, and things are closing from the storm - so casually looking at interesting work things.

Hopefully Salt Lake on Monday should be a little bit clearer, so will take some time back then.

brisk raptor
#

meanwhile in SF

image0.jpg
somber dust
#

Jealous 😭😭

somber dust
#

i am starting to wonder about the Pipeline logs part of the report though
it's quite difficult to read, and i'm generally never using it - and just skipping to the cloud trace (or even the github logs view)
wdyt about replacing it with just a link to the github action logs, since we already have the trace view?

an example: https://github.com/dagger/dagger/actions/runs/11912487630?pr=8995

#

generally really liking the summaries though, with multiple jobs, it gives a very nice summary of each individual job all in one page

somber dust
somber dust
#

did a follow-up in https://github.com/dagger/dagger/pull/8997, looks like the logic i originally added way-back-when that detects if the git repo had changes to it was wrong, and the new cli module somehow triggered it

brisk raptor
brisk raptor
#

Quick update:

  • I missed a gap in the helm modul (thank you Justin for catching it)
  • Helm and all SDKs share a dependency: the release logic.
  • In the full refactor, I solve this by moving those release utilities to the release/ module
  • I'm going to open a separate PR for the bare minimum release/ module, to unblock helm/. We can add more release logic in there as a follow-up, but I know that's a sensitive topic for you Justin so I expect some bikeshedding. Hopefully by keeping it very small in my initial PR, we can avoid holding back the helm merge too long
brisk raptor
#

update: release logic was refactored 3 weeks ago, so that part of my original PR doesn't apply cleanly anymore. Going to try to figure out what the refactor does

#

@somber dust I think that's your refactor. Specifically it's githubRelease() that now receives a dagger.VersionGit

#

I can't understand where that type comes from. The version module? But it has no Git type

#

Oh right it does - it's just in a different file

#

So here we are, I can no longer easily break out githubRelease into a separate module, because it takes a type from another module, which is not allowed

somber dust
#

hm, that is not ideal, i think have a few ideas for how we could do it, and engine changes we could make to unblock it - it's not impossible, just fairly difficult right now

#

up to you for how you want to progress - i've got to head off for the day now, but i've planned and scoped the shell autocompletion work, and expect it to take pretty much all of my time until it's done

brisk raptor
#

Why even carry that Git field around since it's queried directly from the version module?

brisk raptor
somber dust
#

just looking through it

brisk raptor
#

All invocations of githubRelease pass the unmodified dagger.Git field, which itself is received unmodified from calling the version module

#

So I think we can remove that parameter altogether

somber dust
#

yeah, we could probably just call it directly

brisk raptor
#

If later someone needs to parametrize it, they can just add a string. But for now it doesn't even seem that anyone needs it

somber dust
#

yeah, should be pretty doable

brisk raptor
#

OK cool, I will try after my childcare break 🙂 (I'm on PTO today but sick kids at home, so this is my break for the day)

somber dust
#

release module sgtm - or even just a generic "utils" module

#

^ if that helps unblock faster

brisk raptor
#

insert meme: when refactoring CI is your fun breal for the day

#

Thanks Justin! Can't wait to play with autocomplete, it's going to be epic

somber dust
#

😛 yeah, it's gonna be fiddly lol

#

my kinda puzzle

brisk raptor
#

Especially in future v2 when we add daggerverse search + autocomplete of module addresses based on daggerverse index 🤯

somber dust
#

i think we might need to consider rewriting the entire shell parser 😛 (after shipping an initial version)

#

but i'm getting ahead of myself

brisk raptor
brisk raptor
#

well that took longer than I expected... the whole day to be precise

#

but it's pushed

somber dust
#

question - should ci jobs triggered through our modules/gha helper be able to access details of the events that triggered them?

just realized when thinking about @oblique ginkgo's use case in #maintainers message, and also about changing our release pipelines to all use modules/gha (which need to know the tag that triggered them)

i think we should probably support this, would definitely be useful for our use case, and maybe others as well 🙂

#

but not sure what this would look like? a basic idea would be to look for args of a specific "format", and inject those.
e.g. if the target function can take an arg --github-ref, then it should automagically inject GITHUB_REF into it.

somber dust
somber dust
brisk raptor
#

probably a regression across refactors

somber dust
#

yeah the same repositoryUrl issue existed there though

#

since there's not a corresponding git.repository_url

#

but it's not worked since that refactor

#

but i think this logic can just be removed entirely - it's all available in env vars anyways

brisk raptor
#

I don't know what you're referring to, sorry

somber dust
#

i'm saying, i think this entire block of logic probably can just be removed?
i can see what the logic is trying to handle, but github already does this and provides these environment variables
what i'm trying to understand is why this was setup in the first place - was their some case that needed handling that we shouldn't break?

brisk raptor
brisk raptor
#

The use case is hiding GIthub's dumb extra layer of variable interpolation

somber dust
#

i'm so confused

#

we literally use it like this

brisk raptor
#

Mmm you're right - then I can't remember the exact reason I needed it. I think it was a variable that they didn't expose as env vars by default

#

You can also read most of the github context data in environment variables

I think it's that "most" that bit me at some point

#

Also, I used this mechanism for secrets

somber dust
#

the secrets mechanism is slightly different (we do need that bit)

brisk raptor
#

(but I think disabled it in the current version because I was worried it might be too magic)

#

No I mean, I used to auto-select secrets by looking at env var lookups

somber dust
#

you still have to explicitly provide them

brisk raptor
#

I know - I turned it off.

#

It's all in the git history (which you deleted 😛 )

somber dust
#

i think i'd like to remove the github env bits - it won't break anyone, since the behavior is already broken right now
if a user reports a case where it won't work, we'll at least find that case

somber dust
#

we discussed how to do this

brisk raptor
brisk raptor
#

It was a neat trick, and got me excited about using Paul's go-bash library for more things. I wish I could remember the variable that I couldn't get the normal way

somber dust
brisk raptor
#

(at the moment there is no way to mark a module as deprecated or internal, for use by daggerverse. But @gentle orchid is on it I believe as part of #p-daggerverse-quality )

somber dust
#

during our release, looks like i missed the fact that we renamed an arg but it wasn't updated in a workflow

#

not even autogenerating our gha would have helped here

#

is there something that would have helped us catch this? i really like the idea of being able to "statically type" this

#

e.g. if it was somehow possible to say, here's a dagger function call - take it's id, and "replay" the id in the context of gha

#

i might think about this idea

brisk raptor
#

sorry I missed this 👆

#

sounds like a more fundamental problem that we need a Native CI to solve

#

(ie let's get to a point where the gha workflow simply never contains any function name or arguments- because there's only one entry point and everything is configured in the module)

somber dust
#

yeah, for test this is alright, but curious if we have a long-term plan for how this would work for releasing? we need to pass the tag we're releasing in (along with other metadata sometimes, that github populates)

#

if we had contextual env or contextual events or something like that, that would let us do that