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.

Sign up for GitHub

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

Jump to bottom

Allow region change #997

Merged
merged 2 commits into from
Aug 21, 2024
Merged

Allow region change #997

merged 2 commits into from
Aug 21, 2024

Conversation

gordon-klotho
Copy link
Contributor

Preview (no deployed resources, allowed)

klotho ❯ AWS_REGION=us-east-2 go run ./cmd/klotho up -n ./pkg/k2/language_host/python/samples/starter/infra-api.py
██╗  ██╗██╗      ██████╗ ████████╗██╗  ██╗ ██████╗
██║ ██╔╝██║     ██╔═══██╗╚══██╔══╝██║  ██║██╔═══██╗
█████╔╝ ██║     ██║   ██║   ██║   ███████║██║   ██║
██╔═██╗ ██║     ██║   ██║   ██║   ██╔══██║██║   ██║
██║  ██╗███████╗╚██████╔╝   ██║   ██║  ██║╚██████╔╝
╚═╝  ╚═╝╚══════╝ ╚═════╝    ╚═╝   ╚═╝  ╚═╝ ╚═════╝

default-network Success (dry run)
   my-container Success (dry run)
         my-api Success (dry run)
              └ Endpoint: <aws:api_stage:my-api-api:my-api-stage#InvokeUrl>

klotho ❯ AWS_REGION=us-west-2 go run ./cmd/klotho up -n ./pkg/k2/language_host/python/samples/starter/infra-api.py
██╗  ██╗██╗      ██████╗ ████████╗██╗  ██╗ ██████╗
██║ ██╔╝██║     ██╔═══██╗╚══██╔══╝██║  ██║██╔═══██╗
█████╔╝ ██║     ██║   ██║   ██║   ███████║██║   ██║
██╔═██╗ ██║     ██║   ██║   ██║   ██╔══██║██║   ██║
██║  ██╗███████╗╚██████╔╝   ██║   ██║  ██║╚██████╔╝
╚═╝  ╚═╝╚══════╝ ╚═════╝    ╚═╝   ╚═╝  ╚═╝ ╚═════╝

default-network Success (dry run)
   my-container Success (dry run)
         my-api Success (dry run)
              └ Endpoint: <aws:api_stage:my-api-api:my-api-stage#InvokeUrl>

Deploy switch region (disallowed)

klotho ❯ AWS_REGION=us-east-1 go run ./cmd/klotho up ./pkg/k2/language_host/python/samples/func/infra.py
██╗  ██╗██╗      ██████╗ ████████╗██╗  ██╗ ██████╗
██║ ██╔╝██║     ██╔═══██╗╚══██╔══╝██║  ██║██╔═══██╗
█████╔╝ ██║     ██║   ██║   ██║   ███████║██║   ██║
██╔═██╗ ██║     ██║   ██║   ██║   ██╔══██║██║   ██║
██║  ██╗███████╗╚██████╔╝   ██║   ██║  ██║╚██████╔╝
╚═╝  ╚═╝╚══════╝ ╚═════╝    ╚═╝   ╚═╝  ╚═╝ ╚═════╝

my-function         Success
          ├ FunctionName: my-function-function-13c70c2
          └ FunctionArn: arn:aws:lambda:us-west-2:338991950301:function:my-function-function-13c70c2
klotho ❯ AWS_REGION=us-east-2 go run ./cmd/klotho up ./pkg/k2/language_host/python/samples/func/infra.py
██╗  ██╗██╗      ██████╗ ████████╗██╗  ██╗ ██████╗
 894ms ERR > Failed to execute command
  ↳ error=error running up command
          .cause=error resolving initial state
                 .cause=cannot change region (us-west-2 -> us-east-2) with deployed resources: map[my-function:create_complete]
██╗  ██╗██╗      ██████╗ ████████╗██╗  ██╗ ██████╗
██║ ██╔╝██║     ██╔═══██╗╚══██╔══╝██║  ██║██╔═══██╗
█████╔╝ ██║     ██║   ██║   ██║   ███████║██║   ██║
██╔═██╗ ██║     ██║   ██║   ██║   ██╔══██║██║   ██║
██║  ██╗███████╗╚██████╔╝   ██║   ██║  ██║╚██████╔╝
╚═╝  ╚═╝╚══════╝ ╚═════╝    ╚═╝   ╚═╝  ╚═╝ ╚═════╝

exit status 1

atorres-klo
atorres-klo reviewed Aug 19, 2024
View reviewed changes
Copy link
Contributor

@atorres-klo atorres-klo left a comment

Choose a reason for hiding this comment

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

mostly LGTM, 1 question around stale state that we might want to group with this.

cmd/klotho/dependencies.go Show resolved Hide resolved
pkg/k2/orchestration/orchestrator.go
deployed[k] = v.Status
}
}
if len(deployed) > 0 {
Copy link
Contributor

Choose a reason for hiding this comment

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

we may need to couple this with deleting the old pulumi state

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not too familiar with how state works, this should just be mostly the same as it was before. Can you explain a bit more what you mean and why it should delete old pulumi state?

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure, and it may not be pulumi state necessarily (it may be klotho2 state referencing non-existent resources since we never clear that file). We've seen cases where changing region and redeploying causes deploy time errors about vpcs etc. I was thinking we could start with modifying state to clear this out and see if that's enough.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

changing region and redeploying causes deploy time errors

This should only happen if the klotho / pulumi state is out of sync with AWS, right? Since this checks whether any resources are destroyable before allowing the region swap. Any resource that was deployed and not properly cleaned up would still be in a destroyable state I'd think. If not, that sounds like there's a bug somewhere.

If that's that case, for the purposes of this PR, I'm leaning to it being out of scope and we should cut a new card for it.

@gordon-klotho
Copy link
Contributor Author

Tested with up -> down -> change region -> up

klotho ❯ AWS_REGION=us-east-2 klotho up pkg/k2/language_host/python/samples/starter/infra.py
██╗  ██╗██╗      ██████╗ ████████╗██╗  ██╗ ██████╗
██║ ██╔╝██║     ██╔═══██╗╚══██╔══╝██║  ██║██╔═══██╗
█████╔╝ ██║     ██║   ██║   ██║   ███████║██║   ██║
██╔═██╗ ██║     ██║   ██║   ██║   ██╔══██║██║   ██║
██║  ██╗███████╗╚██████╔╝   ██║   ██║  ██║╚██████╔╝
╚═╝  ╚═╝╚══════╝ ╚═════╝    ╚═╝   ╚═╝  ╚═╝ ╚═════╝

default-network         Success
   my-container         Success

klotho ❯ AWS_REGION=us-east-2 klotho down pkg/k2/language_host/python/samples/starter/infra.py
██╗  ██╗██╗      ██████╗ ████████╗██╗  ██╗ ██████╗
██║ ██╔╝██║     ██╔═══██╗╚══██╔══╝██║  ██║██╔═══██╗
█████╔╝ ██║     ██║   ██║   ██║   ███████║██║   ██║
██╔═██╗ ██║     ██║   ██║   ██║   ██╔══██║██║   ██║
██║  ██╗███████╗╚██████╔╝   ██║   ██║  ██║╚██████╔╝
╚═╝  ╚═╝╚══════╝ ╚═════╝    ╚═╝   ╚═╝  ╚═╝ ╚═════╝

   my-container          Success
default-network          Success

klotho ❯ AWS_REGION=ca-central-1 klotho up pkg/k2/language_host/python/samples/starter/infra.py
██╗  ██╗██╗      ██████╗ ████████╗██╗  ██╗ ██████╗
██║ ██╔╝██║     ██╔═══██╗╚══██╔══╝██║  ██║██╔═══██╗
█████╔╝ ██║     ██║   ██║   ██║   ███████║██║   ██║
██╔═██╗ ██║     ██║   ██║   ██║   ██╔══██║██║   ██║
██║  ██╗███████╗╚██████╔╝   ██║   ██║  ██║╚██████╔╝
╚═╝  ╚═╝╚══════╝ ╚═════╝    ╚═╝   ╚═╝  ╚═╝ ╚═════╝

default-network         Success
   my-container         Success

@gordon-klotho gordon-klotho merged commit 1ea5695 into main Aug 21, 2024
6 checks passed
@gordon-klotho gordon-klotho deleted the allow_region_change branch August 21, 2024 15:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DavidSeptimus-Klotho DavidSeptimus-Klotho DavidSeptimus-Klotho approved these changes

@atorres-klo atorres-klo atorres-klo approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@gordon-klotho @atorres-klo @DavidSeptimus-Klotho