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

Refactor: move the application directory up a level #2156

Merged
merged 1 commit into from
Jun 12, 2024
Merged

Conversation

thekaveman
Copy link
Member

@thekaveman thekaveman commented Jun 11, 2024

Part of #2153

Related to:

This PR should be merged after the one in docker-python-web.

From that PR's description:

There are two directories with application code:

  • run/ is created by this image and contains the default nginx and gunicorn configuration
  • app/ is created by the consuming image (benefits or eligibility-server) and contains the application code

This image defines these two directories under /home/calitp/ (the Docker $USERs home).

This PR moves them to the top-level /calitp/ (again, from the Docker $USER).

This change is part of addressing #2153. When we enable the setting WEBSITES_ENABLE_APP_SERVICE_STORAGE=true, /home will be a directory managed by the Azure App Service, so we need the application code out of that directory.

Testing locally

Build the benefits_client, ensuring the docker-python-web digest SHA matches the expected digest SHA: sha256:c937ae01afebab96473045c3a0f3aa9c3a28609d1ff33d756740d2eea254c61f

$ bin/build.sh
=> [client internal] load build definition from Dockerfile                                                                                                              0.0s
 => => transferring dockerfile: xxxB
=> [client internal] load metadata for ghcr.io/cal-itp/docker-python-web:main
...
=> [client  1/10] FROM ghcr.io/cal-itp/docker-python-web:main@sha256:c937ae01afebab96473045c3a0f3aa9c3a28609d1ff33d756740d2eea254c61f
...
=> [client] exporting to image
 => => exporting layers
 => => writing image sha256:xxx
 => => naming to docker.io/library/benefits_client:latest

And restart the devcontainer in VS Code. Then run the app as normal with F5.

@thekaveman thekaveman self-assigned this Jun 11, 2024
@github-actions github-actions bot added docker Application container, devcontainer, Compose, etc. infrastructure Terraform, Azure, etc. labels Jun 11, 2024
Copy link

Coverage report

This PR does not seem to contain any modification to coverable code.

@thekaveman
Copy link
Member Author

thekaveman commented Jun 11, 2024

Front-end tests are failing because they build a container with these changes against docker-python-web:main, without the changes from cal-itp/docker-python-web/pull/42

@thekaveman thekaveman added the chore Chores and tasks for code cleanup, dev experience, admin/configuration settings, etc. label Jun 11, 2024
@thekaveman
Copy link
Member Author

thekaveman commented Jun 11, 2024

terraform plan indicates the expected changes to the storage_account reflecting that the mount location changed from /home/calitp/app/data to /calitp/app/data.

Terraform used the selected providers to generate the following execution
plan. Resource actions are indicated with the following symbols:
  ~ update in-place

Terraform will perform the following actions:

  # azurerm_linux_web_app.main will be updated in-place
  ~ resource "azurerm_linux_web_app" "main" {
        id                                 = "/subscriptions/xxx/resourceGroups/RG-CDT-PUB-VIP-CALITP-D-001/providers/Microsoft.Web/sites/AS-CDT-PUB-VIP-CALITP-D-001"
        name                               = "AS-CDT-PUB-VIP-CALITP-D-001"
        tags                               = { ... }
        # (21 unchanged attributes hidden)

      - storage_account {
          # At least one attribute in this block is (or was) sensitive,
          # so its contents will not be displayed.
        }
      + storage_account {
          # At least one attribute in this block is (or was) sensitive,
          # so its contents will not be displayed.
        }

        # (4 unchanged blocks hidden)
    }

Plan: 0 to add, 1 to change, 0 to destroy.

@thekaveman thekaveman marked this pull request as ready for review June 11, 2024 23:27
@thekaveman thekaveman requested a review from a team as a code owner June 11, 2024 23:27
Copy link
Member

@angela-tran angela-tran left a comment

Choose a reason for hiding this comment

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

This works for me. I was able to open the devcontainer after rebuilding everything 👍

@thekaveman thekaveman merged commit ccb62b2 into dev Jun 12, 2024
12 checks passed
@thekaveman thekaveman deleted the refactor/app-dir branch June 12, 2024 19:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Chores and tasks for code cleanup, dev experience, admin/configuration settings, etc. docker Application container, devcontainer, Compose, etc. infrastructure Terraform, Azure, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants