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

updates for deployment via Unity Marketplace #181

Merged
merged 27 commits into from
Jan 22, 2025
Merged

Conversation

pymonger
Copy link
Contributor

@pymonger pymonger commented Jan 8, 2025

Purpose

This PR updates unity-ads-deployment to support EFS and JupyterHub deployment from the Unity Management Console via the Marketplace while maintaining deployment capability without the MC/Marketplace (e.g. running terraform from laptop).

Proposed Changes

  • [CHANGE] refactored to support MC-injected variables
    • removed symlinking of files that were shared between EFS and Jupyterhub terraform directories to allow for independent deployment via Marketplace
  • [CHANGE] add explicit dependencies to aid in ordering the destruction of AWS resources during terraform destroy
  • [FIX] support deployment without the MC/Marketplace

Issues

Testing

  • See video of live demo showing deployment of EFS and Jupyterhub from the MC/Marketplace (QSR slides)
  • Manually ran terraform on laptop to deploy EFS and Jupyterhub to ensure they can be deployed without the MC/Marketplace

@pymonger pymonger marked this pull request as ready for review January 9, 2025 00:39
@mcduffie mcduffie self-requested a review January 9, 2025 16:26
- also add dependencies to ensure kube config is populated before creating storage classes
@mcduffie
Copy link
Collaborator

I had to make the following modifications to get this working over an existing install:

diff --git a/dev_env/jupyterhub/aws.tf b/dev_env/jupyterhub/aws.tf
index f1afd7d..f429042 100644
--- a/dev_env/jupyterhub/aws.tf
+++ b/dev_env/jupyterhub/aws.tf
@@ -9,3 +9,9 @@ locals {
     Stack = "${var.component_cost_name}"
   }
 }
+
+provider "aws" {
+  default_tags {
+    tags = local.cost_tags
+  }
+}
diff --git a/dev_env/jupyterhub/versions.tf b/dev_env/jupyterhub/versions.tf
index b8e2a5f..bdaccc0 100644
--- a/dev_env/jupyterhub/versions.tf
+++ b/dev_env/jupyterhub/versions.tf
@@ -1,5 +1,4 @@
 terraform {
-  required_version = "~> 1.8.2"
   required_providers {
     aws = {
       source  = "hashicorp/aws"

I know this works with Terraform v1.9.5 aside from a deprecation warning. Can we place modify the required_version to be restrictive?

Also was the default tags local block removed because of the expectation of tags to be passed through a the tags map variable? Is there any home in having default tags. Won't the tags from the variable overwrite them anyways?

@pymonger
Copy link
Contributor Author

I had to make the following modifications to get this working over an existing install:

diff --git a/dev_env/jupyterhub/aws.tf b/dev_env/jupyterhub/aws.tf
index f1afd7d..f429042 100644
--- a/dev_env/jupyterhub/aws.tf
+++ b/dev_env/jupyterhub/aws.tf
@@ -9,3 +9,9 @@ locals {
     Stack = "${var.component_cost_name}"
   }
 }
+
+provider "aws" {
+  default_tags {
+    tags = local.cost_tags
+  }
+}
diff --git a/dev_env/jupyterhub/versions.tf b/dev_env/jupyterhub/versions.tf
index b8e2a5f..bdaccc0 100644
--- a/dev_env/jupyterhub/versions.tf
+++ b/dev_env/jupyterhub/versions.tf
@@ -1,5 +1,4 @@
 terraform {
-  required_version = "~> 1.8.2"
   required_providers {
     aws = {
       source  = "hashicorp/aws"

I know this works with Terraform v1.9.5 aside from a deprecation warning. Can we place modify the required_version to be restrictive?

Also was the default tags local block removed because of the expectation of tags to be passed through a the tags map variable? Is there any home in having default tags. Won't the tags from the variable overwrite them anyways?

@mcduffie: I updated the PR branch with your mods and ran through shared_storage and jupyterhub deployment/tear-down via MC/Marketplace successfully. Let me know if there are other issues from the manual deployment side.

@mcduffie mcduffie merged commit 84b9431 into unity-sds:main Jan 22, 2025
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.

2 participants