add more context to dagger and ci integrations pages #7235
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kpenfound
force-pushed
the
kyle/docs-224-add-more-context-to-dagger-and-ci-integrations-pages
branch
from
367501c
to
f0db273
Compare
|
@d3rp3tt3 you recently used tekton, does this updated version look better? Any more suggestions? |
docs/current_docs/integrations/snippets/tekton-dagger-task.yaml
Outdated
Show resolved
Hide resolved
| @@ -46,7 +43,7 @@ spec: | |||
| script: | | |||
| #!/usr/bin/env sh | |||
| apk add curl | |||
| curl -L https://dl.dagger.io/dagger/install.sh | BIN_DIR=$HOME/.local/bin sh | |||
| curl -L https://dl.dagger.io/dagger/install.sh | BIN_DIR=/usr/local/bin sh | |||
There was a problem hiding this comment.
Just curious about why this change? We use $HOME in the official CLI download instructions
There was a problem hiding this comment.
$HOME is appropriate for workstation installs but not CI. $HOME/.local/bin isn't going to be in the path in CI
vikram-dagger
approved these changes
May 1, 2024
Signed-off-by: kpenfound <[email protected]>
Signed-off-by: kpenfound <[email protected]>
Signed-off-by: kpenfound <[email protected]>
Signed-off-by: Alex Suraci <[email protected]> Signed-off-by: kpenfound <[email protected]>
* engine: consolidate IDs and re-use servers for nested execs This is an internal only refactor, though it fixes a few bugs while also simplifying quite a bit and setting us up for more simplifications soon. The biggest change is that nested execs connect back to the same server as the main client caller rather than being completely independent. * This is required for the fix to services used in modules (separate PR) to fully work * It also should fix the lack of docker auth in many of our integ tests, specifically those that use nested execs, which leads to dockerhub rate limiting Along the way it also does some consolidation of IDs, removing ModuleCallerDigest and just exclusively using ClientID. This requires that we tell module functions and other nested execs which ID to use, but that itself is setup for even more simplifications in follow-ups (we can remove the need for the current DaggerServer construct entirely, among other things). Signed-off-by: Erik Sipsma <[email protected]> * namespace services by server, not by client Previously it was possible to start a dependent service in one module API call, and then use it again in a later call, only to have it fail because it cannot resolve the service address, even though it's still running. This happened because each invocation has its own client ID, and client IDs were used to build service addresses. This change brings service addresses into alignment with the recent change to uniq them by service ID instead of client ID. The overall effect is that services are deduped within a Dagger invocation, even across module calls. So with this change, the service will just stay running and be re-used by a later call, thanks to the grace period. Signed-off-by: Alex Suraci <[email protected]> * fix routing of host services to correct client Signed-off-by: Erik Sipsma <[email protected]> * deregister secret tokens once client disconnects We previously never explicitly removed client ID -> secret token mappings because it theoretically opened more possibilities for malicious attempts to register a client ID with a different token. However, we need to deregister these now since Client IDs are a content hash of the function call/nested exec definition, which means the same client ID can connect and disconnect multiple times per server. The security implications of this also end up being extremely minimal. Registering a client ID with a different secret token was and still is possible *before* a client fully connects. It is possible to after a client disconnects now but this would only amount to a DOS since the "real" client would just be unable to connect. No information would be leaked. It also would have to be in the same server (i.e. a module or nested exec called by the main client directly or transitively). This issue can also be squashed by not leaking the buildkit sock to nested execs/modules, which is possible now by migrating functionality from our shim to our custom executor. There's no immediate plans to do this but the possibility is open whenever needed (or when we make that change for other reasons). Signed-off-by: Erik Sipsma <[email protected]> * add integ test coverage Signed-off-by: Erik Sipsma <[email protected]> --------- Signed-off-by: Erik Sipsma <[email protected]> Signed-off-by: Alex Suraci <[email protected]> Signed-off-by: Erik Sipsma <[email protected]> Co-authored-by: Alex Suraci <[email protected]> Signed-off-by: kpenfound <[email protected]>
Signed-off-by: Vikram Vaswani <[email protected]> Signed-off-by: kpenfound <[email protected]>
Signed-off-by: Vikram Vaswani <[email protected]> Signed-off-by: kpenfound <[email protected]>
* docs: Updates quickstart examples Signed-off-by: Vikram Vaswani <[email protected]> * Fixed capitalization according to new style guide Signed-off-by: Vikram Vaswani <[email protected]> * Updates examples Signed-off-by: Vikram Vaswani <[email protected]> * Copy fix Signed-off-by: Vikram Vaswani <[email protected]> * Added feedback Signed-off-by: Vikram Vaswani <[email protected]> --------- Signed-off-by: Vikram Vaswani <[email protected]> Signed-off-by: kpenfound <[email protected]>
* refactor(sdk/rust): Into -> From Signed-off-by: kjuulh <[email protected]> refactor(sdk/rust): Into -> From Signed-off-by: kjuulh <[email protected]> * chore: remove extra clone Signed-off-by: kjuulh <[email protected]> * chore: fix spacing Signed-off-by: kjuulh <[email protected]> * chore: revert allow warnigns Signed-off-by: kjuulh <[email protected]> --------- Signed-off-by: kjuulh <[email protected]> Signed-off-by: kpenfound <[email protected]>
…agger#7038) * feat: use trait for Struct -> StructID instead of renaming objects Signed-off-by: kjuulh <[email protected]> * chore: include for into id as well Signed-off-by: kjuulh <[email protected]> * chore: remove extra linebreaks Signed-off-by: kjuulh <[email protected]> * chore: fix line breaks Signed-off-by: kjuulh <[email protected]> * refactor: remove direct dependency on ID Signed-off-by: kjuulh <[email protected]> * chore: remove unused tests Signed-off-by: kjuulh <[email protected]> --------- Signed-off-by: kjuulh <[email protected]> Signed-off-by: kpenfound <[email protected]>
Signed-off-by: guangwu <[email protected]> Signed-off-by: kpenfound <[email protected]>
…#7146) Similar to Python SDK, uses introspection from `t.Dagger.instrospection` to generate schema file and mounting it to the container to generate code. Signed-off-by: Thanabodee Charoenpiriyakij <[email protected]> Signed-off-by: kpenfound <[email protected]>
This significantly improves initial directory upload speed, on my computer this goes from 4.8s to a much better 2.5s. *Ideally*, we could also avoid uploading the git directory. This is a bit of a pain *right now* though, so we don't do it! Signed-off-by: Justin Chadwell <[email protected]> Signed-off-by: kpenfound <[email protected]>
* Upgrade `ex_doc` to latest version. * Update documentation on the `Dagger` module. * Remove `getting_started.livemd`, it's quite out-of-date. Signed-off-by: Thanabodee Charoenpiriyakij <[email protected]> Signed-off-by: kpenfound <[email protected]>
Signed-off-by: kpenfound <[email protected]>
Co-authored-by: vikram-dagger <[email protected]> Signed-off-by: Kyle Penfound <[email protected]> Signed-off-by: kpenfound <[email protected]>
This is a very bad default to recommend - essentially this "de-secrets" a value, and this could then be leaked accidentally: - Potentially into OTEL spans - Potentially into cache keys - Potentially into engine logs We should recommend using `WithSecretVariable` instead, which is much better - then rely on the shell to do the interpolation, which avoids all of these problems. The secret scrubber will then guarantee that we don't secrets leaked into logs. Signed-off-by: Justin Chadwell <[email protected]> Signed-off-by: kpenfound <[email protected]>
* fix gitlab before_script Fixes gitlab beore script by installing the Dagger CLI in a location that's availabe in the $PATH Signed-off-by: Marcos Nils <[email protected]> Signed-off-by: kpenfound <[email protected]>
* engine: remove ftp_proxy hack This removes the ftp_proxy hack we've had for a while for passing uncached metadata to containers (previously a lot, but recently trimmed down to just ServerID). Instead, we take advantage of the new custom executor added recently and use that to set the _DAGGER_SERVER_ID env var in containers. The plumbing passes the ServerID via job values, which get read by the solver's ResolveOp callback and used to tell the Executor for ExecOps which ServerID to set in the container. There's quite a bit more cleanup coming to everything involved here, but wanted to get this first step out since that hack, besides being annoying, was kind of dubious in nature and was a suspect in some strange behavior reported sometimes. Signed-off-by: Erik Sipsma <[email protected]> * engine: remove apparently unused opTrackingGateway I don't see that this is actually used or ends up modifying anything. Suspect it's vestigial from pre-OTEL days so removing it. Signed-off-by: Erik Sipsma <[email protected]> * engine: pass worker around instead of custom executor This fixes the problem in the previous commit where Dockerfiles with a syntax pragma hit the executor directly before a ServerID could be set. This wasn't the shortest fix, but it seems to be the best long term one. The end result is that the Worker provided to LLBSolvers is always scoped to the server, so we don't even need to be careful about ensuring the right executor is being used everywhere. It just will be now because it's the only executor available to buildkit internals we use. This required a few adjustments from the previous code: 1. The custom executor has been expanded to also be a custom worker. Fortunately, it almost entirely reuses functionality from upstream's base.Worker, so this doesn't another big chunk of code to maintain. 2. llbSolver is now created in each buildkit.Client and setup with a worker.Controller with the Worker scoped to the current ServerID. 3. Rather than passing the ServerID through job keys, we just pass the whole Worker and use that to resolve ops. 4. Some more general cleanup of worker initialization made possible by these changes. Signed-off-by: Erik Sipsma <[email protected]> * backfill integ test coverage for dockerfile w/ syntax pragma Signed-off-by: Erik Sipsma <[email protected]> * add comment pointing to original runc executor implementation Signed-off-by: Erik Sipsma <[email protected]> --------- Signed-off-by: Erik Sipsma <[email protected]> Signed-off-by: kpenfound <[email protected]>
* sdk(rust): use `load*FromID` in tests Use these newer methods instead of using `container` and `file` to load IDs, which is deprecated and *going to be removed soon*. Signed-off-by: Justin Chadwell <[email protected]> * sdk(go): use load*FromID in tests Signed-off-by: Justin Chadwell <[email protected]> * sdk(typescript): use load*FromID in tests Signed-off-by: Justin Chadwell <[email protected]> * chore: use load*FromID in tests Signed-off-by: Justin Chadwell <[email protected]> --------- Signed-off-by: Justin Chadwell <[email protected]> Signed-off-by: kpenfound <[email protected]>
Signed-off-by: Alex Suraci <[email protected]> Signed-off-by: kpenfound <[email protected]>
* ci: exclude .git dir in ci build Previously, we were using this to generate a work-tree hash, so that new builds would have a different version tag, which helped to solve some weird caching issues with nested execs. However! A problem with this was that we needed to upload *the entire* git directory every time we built - which is an expensive operation, my .git directory for dagger/dagger is 147MB. Instead of relying on git for this, we can use the ID of the input directory! Because of our use of blob sources, a different ID represents a different directory. Additionally, we prefix these new builds with a special "dev-" string to specifically indicate that these are dev builds, with no specific version passed in - these aren't commit hashes, and cannot be treated as such. Signed-off-by: Justin Chadwell <[email protected]> * ci: remove dependency on go sdk The only thing this was being used for here was the connect job - we can just use `--help` to get this info to sanity check it. Signed-off-by: Justin Chadwell <[email protected]> * ci: run linting over ci itself Signed-off-by: Justin Chadwell <[email protected]> --------- Signed-off-by: Justin Chadwell <[email protected]> Signed-off-by: kpenfound <[email protected]>
Signed-off-by: Justin Chadwell <[email protected]> Signed-off-by: kpenfound <[email protected]>
kpenfound
force-pushed
the
kyle/docs-224-add-more-context-to-dagger-and-ci-integrations-pages
branch
from
3a30884
to
fb51d63
Compare
|
ugh the DCO and rebase --sign broke this branch. Making a new one |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.