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

Mutagen (dev mode) improvements #2491

Merged
merged 5 commits into from
Aug 20, 2021
Merged

Mutagen (dev mode) improvements #2491

merged 5 commits into from
Aug 20, 2021

Conversation

edvald
Copy link
Collaborator

@edvald edvald commented Jul 16, 2021

Combining a few related commits, around the mutagen sync mechanism:

improvement(k8s): better process mgmt and logging for dev mode sync
We now poll for more information from the mutagen daemon, and we've overall improved the process management around it.

Besides the improved runtime information, this unblocks more development around this feature, such as running a postSyncCommand (like with the old hot reloading mechanism) and also makes the use of mutagen easier for other contexts like syncing build contexts for in-cluster building.

Also closes #2519 through a few added option flags.

feat(k8s): allow owner/perm tweaks on dev mode syncs
Also updated our docs a bit.

Closes #2519

feat(k8s): add experimental mutagen-based build sync mode
You can now set GARDEN_K8S_BUILD_SYNC_MODE=mutagen to enable a new,
as yet experimental build sync mode, which replaces rsync with mutagen
for synchronizing build contexts ahead of kaniko and
cluster-buildkit builds.

Please give it a spin, since we want to battle-test this a bit before
removing the older rsync method.

@thsig
Copy link
Collaborator

thsig commented Aug 10, 2021

Good stuff!

One thing I noticed: If I run garden deploy --dev vote --env remote in the vote example project, I get the following error:

Failed deploying service 'vote' (from module 'vote'). Here is the output:
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
Command "/Users/ths/.garden/tools/mutagen/62a411c726894601/mutagen sync create /Users/ths/src/garden/examples/vote/vote/src
exec:'/Users/ths/.garden/tools/kubectl/fd63e19492c91ef3/kubectl exec -i --context=gke_garden-dev-200012_europe-west1-b_garden-dev-1
--namespace=vote-ths --container vote Deployment/vote -- /.garden/mutagen-agent synchronizer':/app/src --name Deployment--vote-ths--vote-0
 --auto-start=false --mode two-way-safe" failed with code 1:

Error: unknown flag: --mode
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

@edvald
Copy link
Collaborator Author

edvald commented Aug 10, 2021

Ah, seems the mode flag is not set right. Will fix.

@edvald edvald force-pushed the mutagen-status-messages branch from 4d1321b to 3d320e7 Compare August 10, 2021 19:24
@thsig
Copy link
Collaborator

thsig commented Aug 10, 2021

Another thing I noticed: The Joi validation of devMode.sync[].mode for container modules isn't validating that mode is one of the allowed values.

I was able to set mode: bongo without getting a validation error when re-running the above command.

Copy link
Collaborator

@thsig thsig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! Only had one (minor) additional comment there.

core/src/plugins/kubernetes/dev-mode.ts Show resolved Hide resolved
@edvald edvald force-pushed the mutagen-status-messages branch 2 times, most recently from 0ef23be to c9dd1a5 Compare August 11, 2021 18:25
@edvald edvald changed the title improvement(k8s): better process mgmt and logging for dev mode sync Mutagen (dev mode) improvements Aug 11, 2021
@edvald edvald force-pushed the mutagen-status-messages branch from c9dd1a5 to 0612f32 Compare August 11, 2021 19:56
@edvald
Copy link
Collaborator Author

edvald commented Aug 11, 2021

@thsig did you test this manually? Would you mind kicking it around a bit with GARDEN_K8S_BUILD_SYNC_MODE=mutagen set as well? The integ/e2e tests seem alright, but this is a little tricky to test automatically so we should probably try to break it :P

@edvald edvald force-pushed the mutagen-status-messages branch 4 times, most recently from 8d18080 to fc066c0 Compare August 17, 2021 00:15
edvald added 4 commits August 17, 2021 02:45
We now poll for more information from the mutagen daemon, and we've
overall improved the process management around it.

This unblocks more development around this feature, such as running
a postSyncCommand (like with the old hot reloading mechanism) and also
makes the use of mutagen easier for other contexts like syncing build
contexts for in-cluster building.
You can now set `GARDEN_K8S_BUILD_SYNC_MODE=mutagen` to enable a new,
as yet experimental build sync mode, which replaces rsync with mutagen
for synchronizing build contexts ahead of `kaniko` and
`cluster-buildkit` builds.

Please give it a spin, since we want to battle-test this a bit before
removing the older rsync method.
Just to make it a bit more clear which tests are to run in which
context.
@edvald edvald force-pushed the mutagen-status-messages branch from fc066c0 to 41ad3e0 Compare August 17, 2021 00:46
@thsig thsig force-pushed the mutagen-status-messages branch from 41ad3e0 to 297d8c4 Compare August 20, 2021 08:48
@thsig
Copy link
Collaborator

thsig commented Aug 20, 2021

Yeah, I've poked at this from many different directions—looks good so far 👍

@thsig thsig merged commit c7404c0 into master Aug 20, 2021
@thsig thsig deleted the mutagen-status-messages branch August 20, 2021 09:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEATURE]: Set correct permissions on files synced with devMode.sync
2 participants