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

cloudchamber: Start cloudchamber apply, the new command to deploy #7229

Merged
merged 1 commit into from
Dec 9, 2024

Conversation

gabivlj
Copy link
Contributor

@gabivlj gabivlj commented Nov 11, 2024

container app changes

This command is able to take the [[container-app]] configurations, and deploy them to Cloudchamber.

To render the differences, we are introducing a new dependency with "diff". This was already included in the pnpm lock, however we could consider not rolling out a new dependency into wrangler unless absolutely necessary.

The command is designed to be CI friendly. In the tests there is some example command renders from different kind of configurations.

Fixes #[insert GH or internal issue link(s)].

Describe your change...


  • Tests
    • TODO (before merge)
    • Tests included
    • Tests not necessary because:
  • E2E Tests CI Job required? (Use "e2e" label or ask maintainer to run separately)
    • I don't know
    • Required
    • Not required because: No e2e test for Cloudchamber yet.
  • Public documentation
    • TODO (before merge)
    • Cloudflare docs PR(s):
    • Documentation not necessary because: product is private only for certain users

Copy link

changeset-bot bot commented Nov 11, 2024

🦋 Changeset detected

Latest commit: a6c0a38

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
wrangler Minor
@cloudflare/vitest-pool-workers Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Contributor

github-actions bot commented Nov 11, 2024

A wrangler prerelease is available for testing. You can install this latest build in your project with:

npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12241191766/npm-package-wrangler-7229

You can reference the automatically updated head of this PR with:

npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/prs/7229/npm-package-wrangler-7229

Or you can use npx with this latest build directly:

npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12241191766/npm-package-wrangler-7229 dev path/to/script.js
Additional artifacts:
npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12241191766/npm-package-create-cloudflare-7229 --no-auto-update
npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12241191766/npm-package-cloudflare-kv-asset-handler-7229
npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12241191766/npm-package-miniflare-7229
npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12241191766/npm-package-cloudflare-pages-shared-7229
npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12241191766/npm-package-cloudflare-vitest-pool-workers-7229
npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12241191766/npm-package-cloudflare-workers-editor-shared-7229
npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12241191766/npm-package-cloudflare-workers-shared-7229
npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12241191766/npm-package-cloudflare-workflows-shared-7229

Note that these links will no longer work once the GitHub Actions artifact expires.


wrangler@3.93.0 includes the following runtime dependencies:

Package Constraint Resolved
miniflare workspace:* 3.20241205.0
workerd 1.20241205.0 1.20241205.0
workerd --version 1.20241205.0 2024-12-05

Please ensure constraints are pinned, and miniflare/workerd minor versions match.

@gabivlj gabivlj force-pushed the gv/app-5 branch 5 times, most recently from f3bcfb7 to 649cc88 Compare November 18, 2024 17:14
@gabivlj gabivlj marked this pull request as ready for review November 18, 2024 21:27
@gabivlj gabivlj requested review from a team as code owners November 18, 2024 21:27
@mikenomitch
Copy link
Contributor

Screenshot 2024-11-18 at 11 00 57 PM

Love that this comes up when you run apply without an app in wrangler.toml.

I think"Deploy a container application deploy changes to your application" is a little confusing though. What's the white part vs the orange part? Maybe drop the white part?

@mikenomitch
Copy link
Contributor

Screenshot 2024-11-18 at 11 04 52 PM

Small bug when running the default app then modifying its instance count

@mikenomitch
Copy link
Contributor

Not a blocker: I kind of want a "cloudchamber status" code that can tell me what the status of the apps are in my wrangler.toml and if they're up now/what the diff would be.

@mikenomitch
Copy link
Contributor

mikenomitch commented Nov 19, 2024

I logged out of my account and then ran "cloudchamber apply" on a different account, but the diff it was giving me was from my old account

Screenshot 2024-11-18 at 11 08 04 PM

This should be fresh per account login right? (happy to be challenged on this)

Edit: oh wait... "cloudchamber list" showed the other account, so maybe logout/picking an account didn't work like I expected. Hmmm

@mikenomitch
Copy link
Contributor

Is there a way to unset tier? Should there be?

@mikenomitch
Copy link
Contributor

My dream in using this is to be able to somehow output a wrangler.toml configuration with every possible attribute that I might be able to set and have it add comments for what the possible options are. Docs could do this too, but I don't want to have to find them.

If you hooked up the plumbing, I could provide the template for this.

Not sure what the command would be to do it though "wrangler cloudchamber init --verbose" or something?

Maybe linking to the docs for "wrangler apply" from the "-h" command could be generally useful for describing attrs/showing a verbose toml config?

@mikenomitch
Copy link
Contributor

mikenomitch commented Nov 19, 2024

--json doesn't seem to do anything (at least not on noops or errors)

EDIT: When I got the "name" error above, it didn't work, but in some other case it returned json. Unclear when

@mikenomitch
Copy link
Contributor

Overall, LOVE the overall feel and the workflow, but left some commends - some of which are brainstormy/not blockers.

@mikenomitch
Copy link
Contributor

I've now gotten into a state where I've made a bunch of apps via the CLI and I can't delete them via the CLI because they're app-driven, so even if I remove the deployments they'll come back to life (I think?).

We'll probably need either a "cloudchamber destroy" method that reads from the toml and reverts it all, and/or application CRUD from the CLI. Or maybe some special config to remove an application on next "apply" - thoughts?

@gabivlj
Copy link
Contributor Author

gabivlj commented Nov 19, 2024

Is there a way to unset tier? Should there be?

there should definitely be a way to skip defaults for sure

@gabivlj gabivlj force-pushed the gv/app-5 branch 2 times, most recently from 9550737 to ba38d17 Compare November 22, 2024 23:58
@mikenomitch
Copy link
Contributor

Screenshot 2024-12-02 at 4 43 56 PM

Small bug, when I apply with --json and this config over and over, it auto-applies the config, but then it seems to not actually take, because on the next apply it says Im going to change tier again.

@penalosa
Copy link
Contributor

penalosa commented Dec 3, 2024

@gabivlj Is this ready for review?

@gabivlj
Copy link
Contributor Author

gabivlj commented Dec 3, 2024

@gabivlj Is this ready for review?

Yes. This is up for review. Command designed for heavy Cloudchamber-only users mainly.

My main worry to discuss with the wrangler folks here is the diff dependency we are introducing to detect line changes, we can discuss if we can have another method of detecting these line changes. I also understand that we are moving from TOML to JSON in some cases and this command will need to do that instead of TOML rendering.

.changeset/angry-apricots-swim.md Outdated Show resolved Hide resolved
packages/wrangler/src/config/environment.ts Outdated Show resolved Hide resolved
packages/wrangler/src/config/environment.ts Outdated Show resolved Hide resolved
packages/wrangler/src/cloudchamber/apply.ts Show resolved Hide resolved
packages/wrangler/src/cloudchamber/apply.ts Outdated Show resolved Hide resolved
@gabivlj gabivlj force-pushed the gv/app-5 branch 2 times, most recently from be87569 to 21b343b Compare December 3, 2024 22:54
container app changes

This command is able to take the [[container-app]] configurations, and
deploy them to Cloudchamber.

To render the differences, we are basing off from https://www.npmjs.com/package/diff?activeTab=readme
but transformed to TS and without adding a 600kb bundle size.

The command is designed to be CI friendly. In the tests there is some
example command renders from different kind of configurations.

One of the biggest TODOs here is proper error rendering. We hope to
improve that overtime, and pinpoint to the user in the wrangler.toml
what went wrong.
Copy link
Contributor

@penalosa penalosa left a comment

Choose a reason for hiding this comment

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

Approved from the Wrangler side pending dropping the diff dependency (which I think you've done?) and copying the config type into the config validation file

packages/wrangler/src/config/environment.ts Outdated Show resolved Hide resolved
@gabivlj gabivlj force-pushed the gv/app-5 branch 5 times, most recently from 8b25786 to a6c0a38 Compare December 9, 2024 17:41
@gabivlj gabivlj merged commit 669d7ad into main Dec 9, 2024
29 checks passed
@gabivlj gabivlj deleted the gv/app-5 branch December 9, 2024 18:52
@workers-devprod workers-devprod mentioned this pull request Dec 9, 2024
penalosa pushed a commit that referenced this pull request Jan 10, 2025
…7229)

container app changes

This command is able to take the [[container-app]] configurations, and
deploy them to Cloudchamber.

To render the differences, we are basing off from https://www.npmjs.com/package/diff?activeTab=readme
but transformed to TS and without adding a 600kb bundle size.

The command is designed to be CI friendly. In the tests there is some
example command renders from different kind of configurations.

One of the biggest TODOs here is proper error rendering. We hope to
improve that overtime, and pinpoint to the user in the wrangler.toml
what went wrong.
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.

3 participants