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

SS-1191: Bugfix related to environment and flavor deletion #243

Merged
merged 38 commits into from
Dec 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
9c6ddf4
bugfix SS-1191 - environment and flavor deletion
akochari Oct 21, 2024
91bc0b6
better naming in the loop
akochari Oct 22, 2024
4d0eaeb
better naming in the loop
akochari Oct 22, 2024
83d860c
remove commented out line
akochari Oct 22, 2024
8d684ca
Merge branch 'develop' into env-and-flavor-deletion-fixes
akochari Oct 22, 2024
9ca5daf
remove unneccesary logging statement
akochari Oct 22, 2024
27c2f72
Merge branch 'develop' into env-and-flavor-deletion-fixes
akochari Nov 19, 2024
c9418bc
allow instructions for local docker for gradio and streamlit apps (sm…
akochari Nov 19, 2024
ac5585c
Merge branch 'develop' into env-and-flavor-deletion-fixes
akochari Nov 20, 2024
576069a
prohibit deleting environments when they are in use by an app
akochari Nov 21, 2024
e023356
general function for checking if flavor or env can be deleted
akochari Nov 21, 2024
9a93db4
add e2e tests for flavor and environment deletion functionality
akochari Nov 21, 2024
c1897dd
precommit fix
akochari Nov 21, 2024
367d4c8
Merge branch 'develop' into env-and-flavor-deletion-fixes
akochari Nov 21, 2024
d4c39fe
manual logout and login where cypress doesn't want to get back to a s…
akochari Nov 22, 2024
0fc1629
migration for changes to environment field
akochari Nov 22, 2024
fdcc6b5
remove unused bits of code
akochari Nov 26, 2024
989d1be
add unit tests for flavor creation
akochari Nov 26, 2024
bdc61d1
simplify function to check if ok to delete
akochari Nov 26, 2024
476c1bc
add flavor deletion tests
akochari Nov 26, 2024
3120ce4
pre-commit fix
akochari Nov 26, 2024
0618247
include more flavor deletion tests
akochari Nov 26, 2024
d228984
prohibit flavor and environment editing for regular users
akochari Nov 26, 2024
29e1559
Merge branch 'develop' into env-and-flavor-deletion-fixes
akochari Nov 26, 2024
dc7e594
add unit tests for environment creation and deletion
akochari Nov 27, 2024
b93efc7
Merge branch 'develop' into env-and-flavor-deletion-fixes
akochari Nov 27, 2024
b1dd402
Merge branch 'develop' into env-and-flavor-deletion-fixes
akochari Nov 27, 2024
96e3843
add type hints for the new function
akochari Dec 6, 2024
7d64d26
merge changes from develop branch
akochari Dec 6, 2024
dbff588
Merge branch 'develop' into env-and-flavor-deletion-fixes
akochari Dec 6, 2024
001a489
updated serve logo
akochari Dec 9, 2024
8c984fa
remove extra migrations file
akochari Dec 9, 2024
0d2c73f
split unit tests into smaller tests
akochari Dec 9, 2024
a322fb9
fix precommit
akochari Dec 9, 2024
8a9af69
uppercase names of global variables
akochari Dec 10, 2024
02b6b50
uppercase names for global variables
akochari Dec 10, 2024
fafe0db
follow redirects in tests
akochari Dec 10, 2024
a4e9243
Merge branch 'develop' into env-and-flavor-deletion-fixes
akochari Dec 10, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion apps/models/app_types/jupyter.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ class JupyterInstance(BaseAppInstance):
)
volume = models.ManyToManyField("VolumeInstance", blank=True)
access = models.CharField(max_length=20, default="project", choices=ACCESS_TYPES)
environment: Environment = models.ForeignKey(Environment, on_delete=models.DO_NOTHING, null=True, blank=True)
environment: Environment = models.ForeignKey(Environment, on_delete=models.RESTRICT, null=True, blank=True)

def get_k8s_values(self):
k8s_values = super().get_k8s_values()
Expand Down
2 changes: 1 addition & 1 deletion apps/models/app_types/rstudio.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ class RStudioInstance(BaseAppInstance):
)
volume = models.ManyToManyField("VolumeInstance", blank=True)
access = models.CharField(max_length=20, default="project", choices=ACCESS_TYPES)
environment: Environment = models.ForeignKey(Environment, on_delete=models.DO_NOTHING, null=True, blank=True)
environment: Environment = models.ForeignKey(Environment, on_delete=models.RESTRICT, null=True, blank=True)

def get_k8s_values(self):
k8s_values = super().get_k8s_values()
Expand Down
124 changes: 72 additions & 52 deletions cypress/e2e/ui-tests/test-superuser-functionality.cy.js
Original file line number Diff line number Diff line change
Expand Up @@ -164,13 +164,11 @@ describe("Test superuser access", () => {
cy.get('#submit-id-submit').contains('Submit').click()
cy.get('tr:contains("' + private_app_name_2 + '")').should('exist') // regular user's private app now has a different name

//cy.wait(10000) // Not needed because of the retryability built into cypress.
cy.get('tr:contains("' + private_app_name_2 + '")', {timeout: longCmdTimeoutMs}).find('span', {timeout: longCmdTimeoutMs}).should('contain', 'Running') // add this because to make sure the app is running before deleting otherwise it gives an error,
cy.logf("Deleting a regular user's private app", Cypress.currentTest)
cy.get('tr:contains("' + private_app_name_2 + '")').find('i.bi-three-dots-vertical').click()
cy.get('tr:contains("' + private_app_name_2 + '")').find('a.confirm-delete').click()
cy.get('button').contains('Delete').click()
//cy.wait(5000) // Not needed because of the retryability built into cypress.
cy.get('tr:contains("' + private_app_name_2 + '")', {timeout: longCmdTimeoutMs}).find('span', {timeout: longCmdTimeoutMs}).should('contain', 'Deleted')

cy.logf("Deleting a regular user's project", Cypress.currentTest)
Expand All @@ -188,7 +186,9 @@ describe("Test superuser access", () => {
// Names of objects to create
const project_name = "e2e-proj-flavor-env-test"
const new_flavor_name = "4 CPU, 8 GB RAM"
const new_flavor_name_unused = "Unused flavor"
const new_environment_name = "e2e test environment"
const new_environment_name_unused = "Unused environment"

cy.logf("Logging in as a regular user and creating a project", Cypress.currentTest)
cy.fixture('users.json').then(function (data) {
Expand All @@ -200,7 +200,6 @@ describe("Test superuser access", () => {
cy.get("a").contains('Create').first().click()
cy.get('input[name=name]').type(project_name)
cy.get("input[name=save]").contains('Create project').click()
//cy.wait(5000) // sometimes it takes a while to create a project. Not needed because of cypress retryability.
cy.get('h3').should('contain', project_name)

Cypress.session.clearAllSavedSessions()
Expand All @@ -210,7 +209,7 @@ describe("Test superuser access", () => {
cy.loginViaUI(users.superuser.email, users.superuser.password)
})

cy.logf("Creating a new flavor in the regular user's project", Cypress.currentTest)
cy.logf("Creating new flavors in the regular user's project", Cypress.currentTest)
cy.visit("/projects/")
cy.contains('.card-title', project_name).parents('.card-body').siblings('.card-footer').find('a:contains("Open")').first().click()
cy.get('[data-cy="settings"]').click()
Expand All @@ -222,7 +221,15 @@ describe("Test superuser access", () => {
cy.get('input[name="mem_lim"]').clear().type("8Gi")
cy.get('button').contains("Create flavor").click()

cy.logf("Creating a new Jupyter Lab environment in the regular user's project", Cypress.currentTest)
cy.get('.list-group').find('a').contains('Flavors').click()
cy.get('input[name="flavor_name"]').type(new_flavor_name_unused)
cy.get('input[name="cpu_req"]').clear().type("500m")
cy.get('input[name="cpu_lim"]').clear().type("4000m")
cy.get('input[name="mem_req"]').clear().type("2Gi")
cy.get('input[name="mem_lim"]').clear().type("8Gi")
cy.get('button').contains("Create flavor").click()

cy.logf("Creating new Jupyter Lab environments in the regular user's project", Cypress.currentTest)
cy.visit("/projects/")
cy.contains('.card-title', project_name).parents('.card-body').siblings('.card-footer').find('a:contains("Open")').first().click()
cy.get('[data-cy="settings"]').click()
Expand All @@ -233,12 +240,19 @@ describe("Test superuser access", () => {
cy.get('#environment_app').select('Jupyter Lab')
cy.get('button').contains("Create environment").click()

Cypress.session.clearAllSavedSessions()
cy.logf("Logging back in as a regular user and using the new flavor and environment", Cypress.currentTest)
cy.get('.list-group').find('a').contains('Environments').click()
cy.get('input[name="environment_name"]').type(new_environment_name_unused)
cy.get('input[name="environment_repository"]').clear().type("docker.io")
cy.get('input[name="environment_image"]').clear().type("jupyter/minimal-notebook:latest")
cy.get('#environment_app').select('Jupyter Lab')
cy.get('button').contains("Create environment").click()

const createResources = Cypress.env('create_resources');

if (createResources === true) {

Cypress.session.clearAllSavedSessions()
cy.logf("Logging back in as a regular user and using the new flavor and environment", Cypress.currentTest)
cy.fixture('users.json').then(function (data) {
users = data
cy.loginViaUI(users.superuser_testuser.email, users.superuser_testuser.password)
Expand Down Expand Up @@ -311,6 +325,56 @@ describe("Test superuser access", () => {
cy.get('tr:contains("' + app_name_env + '")').find('a').contains('Settings').click()
cy.get('#id_environment').find(':selected').should('contain', new_environment_name)

cy.logf("Checking that admin cannot delete flavor or environment currently in use", Cypress.currentTest)
cy.logf("Logging in as a superuser", Cypress.currentTest)
// I do this logout and login manually (rather than using Cypress sessions) because Cypress
// refused to do one more session for this user here for some reason
cy.get('button.btn-profile').contains("Profile").click()
cy.get('li.btn-group').find('button').contains("Sign out").click()
cy.get("title").should("have.text", "Logout | SciLifeLab Serve (beta)")
cy.fixture('users.json').then(function (data) {
users = data
cy.visit('/accounts/login/')
cy.get('input[name=username]').type(users.superuser.email)
cy.get('input[name=password]').type(`${users.superuser.password}{enter}`, { log: false })
cy.url().should('include', '/projects')
cy.get('h3').should('contain', 'My projects')
})

cy.logf("Trying to delete a flavor that was used", Cypress.currentTest)
cy.visit("/projects/")
cy.contains('.card-title', project_name).parents('.card-body').siblings('.card-footer').find('a:contains("Open")').first().click()
cy.get('[data-cy="settings"]').click()
cy.logf("Deleting a flavor that was used", Cypress.currentTest)
cy.get('.list-group').find('a').contains('Flavors').click()
cy.get('#flavor_pk').select(new_flavor_name)
cy.get('button').contains("Delete flavor").click()
cy.get('div.alert-danger').contains('Flavor cannot be deleted').should('exist')

cy.logf("Deleting a flavor that was not used", Cypress.currentTest)
cy.logf("Trying flavor deletion", Cypress.currentTest)
cy.get('.list-group').find('a').contains('Flavors').click()
cy.get('#flavor_pk').select(new_flavor_name_unused)
cy.get('button').contains("Delete flavor").click()
cy.get('.list-group').find('a').contains('Flavors').click()
cy.get('#flavor_pk').contains(new_flavor_name_unused).should("not.exist")

cy.logf("Trying to delete an environment that was used", Cypress.currentTest)
cy.visit("/projects/")
cy.contains('.card-title', project_name).parents('.card-body').siblings('.card-footer').find('a:contains("Open")').first().click()
cy.get('[data-cy="settings"]').click()
cy.logf("Deleting a flavor that was used", Cypress.currentTest)
cy.get('.list-group').find('a').contains('Environments').click()
cy.get('#environment_pk').select(new_environment_name)
cy.get('button').contains("Delete environment").click()
cy.get('div.alert-danger').contains('Environment cannot be deleted').should('exist')

cy.logf("Deleting an environment that was not used", Cypress.currentTest)
cy.get('.list-group').find('a').contains('Environments').click()
cy.get('#environment_pk').select(new_environment_name_unused)
cy.get('button').contains("Delete environment").click()
cy.get('.list-group').find('a').contains('Environments').click()
cy.get('#environment_pk').contains(new_environment_name_unused).should("not.exist")

} else {
cy.logf('Skipped because create_resources is not true', Cypress.currentTest);
Expand All @@ -327,55 +391,11 @@ describe("Test superuser access", () => {
})
})

it.skip("can create a persistent volume", () => {
// This test is not used, since creating PVCs like this is not the correct way any more.
// The correct way is to create a volume in the admin panel.

// Names of objects to create
const project_name_pvc = "e2e-superuser-pvc-test"
const volume_name = "e2e-project-vol"

cy.logf("Creating a blank project", Cypress.currentTest)
cy.createBlankProject(project_name_pvc)

cy.logf("Creating a persistent volume", Cypress.currentTest)
cy.visit("/projects/")
cy.contains('.card-title', project_name_pvc).parents('.card-body').siblings('.card-footer').find('a:contains("Open")').first().click()

cy.get('div.card-body:contains("Persistent Volume")').find('a:contains("Create")').click()
cy.get('#id_name').type(volume_name)
cy.get('#submit-id-submit').contains('Submit').click()
cy.get('tr:contains("' + volume_name + '")').should('exist') // persistent volume has been created

// This does not work in our CI. Disabled for now, needs to be enabled for runs against an instance of Serve running on the cluster
/*
cy.get('tr:contains("' + volume_name + '")').find('span').should('contain', 'Installed') // confirm the volume is working

cy.log("Deleting the created persistent volume")
cy.get('tr:contains("' + volume_name + '")').find('i.bi-three-dots-vertical').click()
cy.get('tr:contains("' + volume_name + '")').find('a.confirm-delete').click()
cy.get('button').contains('Delete').click()
cy.get('tr:contains("' + volume_name + '")').find('span').should('contain', 'Deleted') // confirm the volume has been deleted
*/

cy.logf("Deleting the created project", Cypress.currentTest)
cy.visit("/projects/")
cy.contains('.card-title', project_name_pvc).parents('.card-body').siblings('.card-footer').find('.confirm-delete').click()
.then((href) => {
cy.get('div#modalConfirmDelete').should('have.css', 'display', 'block')
cy.get("h1#modalConfirmDeleteLabel").then(function($elem) {
cy.get('div#modalConfirmDeleteFooter').find('button').contains('Delete').click()
cy.contains(project_name_pvc).should('not.exist') // confirm the project has been deleted
})
})

})

it("can bypass N projects limit", () => {
// Names of projects to create
const project_name = "e2e-superuser-proj-limits-test"

cy.logf("Create 10 projects (current limit for regular users)", Cypress.currentTest)
cy.logf("Create 10 projects (current limit for regular users) with the same name (currently not possible for regular users to use the same name)", Cypress.currentTest)
Cypress._.times(10, () => {
// better to write this out rather than use the createBlankProject command because then we can do a 5000 ms pause only once
cy.visit("/projects/")
Expand Down
137 changes: 137 additions & 0 deletions projects/tests/test_create_delete_environments.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
from django.contrib.auth import get_user_model
from django.contrib.messages import get_messages
from django.test import TestCase

from apps.models import Apps, JupyterInstance, Subdomain
from projects.models import Environment, Project
from projects.views import can_model_instance_be_deleted

User = get_user_model()

TEST_USER = {"username": "foo1", "email": "[email protected]", "password": "bar"}
TEST_SUPERUSER = {"username": "superuser", "email": "[email protected]", "password": "bar"}


class EnvironmentTestCaseRegularUser(TestCase):
def setUp(self):
user = User.objects.create_user(TEST_USER["username"], TEST_USER["email"], TEST_USER["password"])
self.project = Project.objects.create_project(name="test-env", owner=user, description="")
User.objects.create_superuser(TEST_SUPERUSER["username"], TEST_SUPERUSER["email"], TEST_SUPERUSER["password"])
self.app = Apps.objects.create(name="Some App", slug="someapp")
self.env_to_be_deleted = Environment.objects.create(
app=self.app, name="env-to-be-deleted", project=self.project
)
self.client.login(username=TEST_USER["email"], password=TEST_USER["password"])

def test_environment_creation_regular_user(self):
"""
Test regular user not allowed to create environment
"""
n_envs_before = Environment.objects.count()
response = self.client.post(
f"/projects/{self.project.slug}/createenvironment/",
{
"environment_name": "new-env-user",
"environment_repository": "n",
"environment_image": "n",
"environment_app": "n",
"app": self.app.pk,
},
)
self.assertEqual(response.status_code, 403)
n_envs_after = Environment.objects.count()
self.assertEqual(n_envs_before, n_envs_after)

def test_environment_deletion_regular_user(self):
"""
Test regular user not allowed to delete environment
"""
akochari marked this conversation as resolved.
Show resolved Hide resolved
n_envs_before = Environment.objects.count()
response = self.client.post(
f"/projects/{self.project.slug}/deleteenvironment/", {"environment_pk": self.env_to_be_deleted.pk}
)
self.assertEqual(response.status_code, 403)
n_envs_after = Environment.objects.count()
self.assertEqual(n_envs_before, n_envs_after)


class EnvironmentTestCaseSuperUser(TestCase):
def setUp(self):
self.user = User.objects.create_user(TEST_USER["username"], TEST_USER["email"], TEST_USER["password"])
self.project = Project.objects.create_project(name="test-env", owner=self.user, description="")
User.objects.create_superuser(TEST_SUPERUSER["username"], TEST_SUPERUSER["email"], TEST_SUPERUSER["password"])
self.app = Apps.objects.create(name="Some App", slug="someapp")
self.env_to_be_deleted = Environment.objects.create(
app=self.app, name="env-to-be-deleted", project=self.project
)
self.client.login(username=TEST_SUPERUSER["email"], password=TEST_SUPERUSER["password"])

def test_environment_creation_superuser(self):
"""
Test superuser is allowed to create environment
"""
n_envs_before = Environment.objects.count()
response = self.client.post(
f"/projects/{self.project.slug}/createenvironment/",
{
"environment_name": "new-env-superuser",
"environment_repository": "n",
"environment_image": "n",
"environment_app": self.app.pk,
},
follow=True,
)
self.assertEqual(response.status_code, 200)
n_envs_after = Environment.objects.count()
self.assertEqual(n_envs_before + 1, n_envs_after)

def test_environment_deletion_inuse_superuser(self):
"""
Test it is not allowed to delete environment that is in use
"""
env_cannot_be_deleted = Environment.objects.create(
app=self.app, name="env-cannot-be-deleted", project=self.project
)
subdomain = Subdomain.objects.create(subdomain="test_internal")
self.app_instance = JupyterInstance.objects.create(
access="public",
owner=self.user,
name="test_app_instance_env",
app=self.app,
project=self.project,
environment=env_cannot_be_deleted,
subdomain=subdomain,
)
can_env_be_deleted = can_model_instance_be_deleted("environment", env_cannot_be_deleted)
self.assertFalse(can_env_be_deleted)

n_envs_before = Environment.objects.count()
response = self.client.post(
f"/projects/{self.project.slug}/deleteenvironment/",
{"environment_pk": env_cannot_be_deleted.pk},
follow=True,
)
self.assertEqual(response.status_code, 200)
messages = list(get_messages(response.wsgi_request))
self.assertEqual(len(messages), 1)
self.assertIn("cannot be deleted", str(messages[0]))
n_envs_after = Environment.objects.count()

self.assertEqual(n_envs_before, n_envs_after)

def test_environment_deletion_notinuse_superuser(self):
"""
Test it is allowed to delete environment that is not in use
"""
can_env_be_deleted = can_model_instance_be_deleted("environment", self.env_to_be_deleted)
self.assertTrue(can_env_be_deleted)
n_envs_before = Environment.objects.count()
response = self.client.post(
f"/projects/{self.project.slug}/deleteenvironment/",
{"environment_pk": self.env_to_be_deleted.pk},
follow=True,
)
self.assertEqual(response.status_code, 200)
n_envs_after = Environment.objects.count()

self.assertEqual(n_envs_before - 1, n_envs_after)
Loading