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

🎨 efs improvements (group extra properties) 🗃️ #6493

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
"""add `enable_efs` to group extra properties

Revision ID: ea3952fe5a0e
Revises: 8a742f3efdd9
Create Date: 2024-10-07 06:24:42.464942+00:00

"""
import sqlalchemy as sa
from alembic import op

# revision identifiers, used by Alembic.
revision = "ea3952fe5a0e"
down_revision = "8a742f3efdd9"
branch_labels = None
depends_on = None


def upgrade():
# ### commands auto generated by Alembic - please adjust! ###
op.add_column(
"groups_extra_properties",
sa.Column(
"enable_efs", sa.Boolean(), server_default=sa.text("false"), nullable=False
),
)
# ### end Alembic commands ###


def downgrade():
# ### commands auto generated by Alembic - please adjust! ###
op.drop_column("groups_extra_properties", "enable_efs")
# ### end Alembic commands ###
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,13 @@
server_default=sa.sql.expression.false(),
doc="If true, will send telemetry for new style dynamic services to frontend",
),
sa.Column(
"enable_efs",
matusdrobuliak66 marked this conversation as resolved.
Show resolved Hide resolved
sa.Boolean(),
nullable=False,
server_default=sa.sql.expression.false(),
doc="If true, will mount efs distributed file system when dynamic services starts",
),
sa.UniqueConstraint(
"group_id", "product_name", name="group_id_product_name_uniqueness"
),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ class GroupExtraProperties(FromRowMixin):
enable_telemetry: bool
created: datetime.datetime
modified: datetime.datetime
enable_efs: bool


async def _list_table_entries_ordered_by_group_type(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
from pydantic import BaseModel
from simcore_postgres_database.utils_groups_extra_properties import (
GroupExtraProperties,
GroupExtraPropertiesRepo,
Expand All @@ -6,6 +7,12 @@
from ._base import BaseRepository


class UserExtraProperties(BaseModel):
is_internet_enabled: bool
is_telemetry_enabled: bool
is_efs_enabled: bool


class GroupsExtraPropertiesRepository(BaseRepository):
async def _get_aggregated_properties_for_user(
self,
Expand All @@ -31,3 +38,15 @@ async def is_telemetry_enabled(self, *, user_id: int, product_name: str) -> bool
)
telemetry_enabled: bool = group_extra_properties.enable_telemetry
return telemetry_enabled

async def get_user_extra_properties(
matusdrobuliak66 marked this conversation as resolved.
Show resolved Hide resolved
self, *, user_id: int, product_name: str
) -> UserExtraProperties:
group_extra_properties = await self._get_aggregated_properties_for_user(
user_id=user_id, product_name=product_name
)
return UserExtraProperties(
is_internet_enabled=group_extra_properties.internet_access,
is_telemetry_enabled=group_extra_properties.enable_telemetry,
is_efs_enabled=group_extra_properties.enable_efs,
)
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
from ....core.dynamic_services_settings.sidecar import DynamicSidecarSettings
from ....core.settings import AppSettings
from ....models.dynamic_services_scheduler import SchedulerData
from ....modules.db.repositories.groups_extra_properties import UserExtraProperties
from .._namespace import get_compose_namespace
from ..volumes import DynamicSidecarVolumesPathsResolver
from ._constants import DOCKER_CONTAINER_SPEC_RESTART_POLICY_DEFAULTS
Expand Down Expand Up @@ -220,6 +221,7 @@ async def _get_mounts(
app_settings: AppSettings,
has_quota_support: bool,
rpc_client: RabbitMQRPCClient,
is_efs_enabled: bool,
) -> list[dict[str, Any]]:
mounts: list[dict[str, Any]] = [
# docker socket needed to use the docker api
Expand Down Expand Up @@ -270,18 +272,9 @@ async def _get_mounts(
)
)

# We check whether user has access to EFS feature
use_efs = False
efs_settings = dynamic_sidecar_settings.DYNAMIC_SIDECAR_EFS_SETTINGS
if (
efs_settings
and scheduler_data.user_id in efs_settings.EFS_ONLY_ENABLED_FOR_USERIDS
):
use_efs = True

# state paths now get mounted via different driver and are synced to s3 automatically
for path_to_mount in scheduler_data.paths_mapping.state_paths:
if use_efs:
if is_efs_enabled:
assert dynamic_sidecar_settings.DYNAMIC_SIDECAR_EFS_SETTINGS # nosec

_storage_directory_name = DynamicSidecarVolumesPathsResolver.volume_name(
Expand Down Expand Up @@ -411,10 +404,9 @@ async def get_dynamic_sidecar_spec( # pylint:disable=too-many-arguments# noqa:
app_settings: AppSettings,
*,
has_quota_support: bool,
allow_internet_access: bool,
hardware_info: HardwareInfo | None,
metrics_collection_allowed: bool,
telemetry_enabled: bool,
user_extra_properties: UserExtraProperties,
rpc_client: RabbitMQRPCClient,
) -> AioDockerServiceSpec:
"""
Expand All @@ -434,6 +426,7 @@ async def get_dynamic_sidecar_spec( # pylint:disable=too-many-arguments# noqa:
app_settings=app_settings,
has_quota_support=has_quota_support,
rpc_client=rpc_client,
is_efs_enabled=user_extra_properties.is_efs_enabled,
)

ports = _get_ports(
Expand Down Expand Up @@ -512,9 +505,9 @@ async def get_dynamic_sidecar_spec( # pylint:disable=too-many-arguments# noqa:
compose_namespace,
scheduler_data,
app_settings,
allow_internet_access=allow_internet_access,
allow_internet_access=user_extra_properties.is_internet_enabled,
metrics_collection_allowed=metrics_collection_allowed,
telemetry_enabled=telemetry_enabled,
telemetry_enabled=user_extra_properties.is_telemetry_enabled,
),
"Hosts": [],
"Image": dynamic_sidecar_settings.DYNAMIC_SIDECAR_IMAGE,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,8 @@ async def action(cls, app: FastAPI, scheduler_data: SchedulerData) -> None:
groups_extra_properties = get_repository(app, GroupsExtraPropertiesRepository)

assert scheduler_data.product_name is not None # nosec
allow_internet_access: bool = await groups_extra_properties.has_internet_access(

user_extra_properties = await groups_extra_properties.get_user_extra_properties(
user_id=scheduler_data.user_id, product_name=scheduler_data.product_name
)

Expand All @@ -194,7 +195,7 @@ async def action(cls, app: FastAPI, scheduler_data: SchedulerData) -> None:
"uuid": f"{scheduler_data.node_uuid}", # needed for removal when project is closed
},
"Attachable": True,
"Internal": not allow_internet_access,
"Internal": not user_extra_properties.is_internet_enabled,
}
dynamic_sidecar_network_id = await create_network(network_config)

Expand All @@ -217,11 +218,6 @@ async def action(cls, app: FastAPI, scheduler_data: SchedulerData) -> None:
# generate a new `run_id` to avoid resource collisions
scheduler_data.run_id = RunID.create()

# telemetry configuration
is_telemetry_enabled = await groups_extra_properties.is_telemetry_enabled(
user_id=scheduler_data.user_id, product_name=scheduler_data.product_name
)

rpc_client: RabbitMQRPCClient = app.state.rabbitmq_rpc_client

# WARNING: do NOT log, this structure has secrets in the open
Expand All @@ -235,9 +231,8 @@ async def action(cls, app: FastAPI, scheduler_data: SchedulerData) -> None:
app_settings=app.state.settings,
hardware_info=scheduler_data.hardware_info,
has_quota_support=dynamic_services_scheduler_settings.DYNAMIC_SIDECAR_ENABLE_VOLUME_LIMITS,
allow_internet_access=allow_internet_access,
metrics_collection_allowed=metrics_collection_allowed,
telemetry_enabled=is_telemetry_enabled,
user_extra_properties=user_extra_properties,
rpc_client=rpc_client,
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@
)
from simcore_service_director_v2.models.dynamic_services_scheduler import SchedulerData
from simcore_service_director_v2.modules.catalog import CatalogClient
from simcore_service_director_v2.modules.db.repositories.groups_extra_properties import (
UserExtraProperties,
)
from simcore_service_director_v2.modules.dynamic_sidecar.docker_service_specs import (
get_dynamic_sidecar_spec,
)
Expand Down Expand Up @@ -451,9 +454,12 @@ async def test_get_dynamic_proxy_spec(
app_settings=minimal_app.state.settings,
hardware_info=hardware_info,
has_quota_support=False,
allow_internet_access=False,
metrics_collection_allowed=True,
telemetry_enabled=True,
user_extra_properties=UserExtraProperties(
is_internet_enabled=False,
is_telemetry_enabled=True,
is_efs_enabled=False,
),
rpc_client=Mock(),
)

Expand Down Expand Up @@ -546,9 +552,12 @@ async def test_merge_dynamic_sidecar_specs_with_user_specific_specs(
app_settings=minimal_app.state.settings,
hardware_info=hardware_info,
has_quota_support=False,
allow_internet_access=False,
metrics_collection_allowed=True,
telemetry_enabled=True,
user_extra_properties=UserExtraProperties(
is_internet_enabled=False,
is_telemetry_enabled=True,
is_efs_enabled=False,
),
rpc_client=Mock(),
)
assert dynamic_sidecar_spec
Expand Down
8 changes: 3 additions & 5 deletions services/efs-guardian/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -168,14 +168,12 @@ ENV SC_BUILD_TARGET=production \
ENV PYTHONOPTIMIZE=TRUE

WORKDIR /home/efs
# ensure home folder is read/writable for user efs
RUN chown -R efs /home/efs

# Starting from clean base image, copies pre-installed virtualenv from prod-only-deps
COPY --chown=efs:efs --from=prod-only-deps ${VIRTUAL_ENV} ${VIRTUAL_ENV}
COPY --from=prod-only-deps ${VIRTUAL_ENV} ${VIRTUAL_ENV}

# Copies booting scripts
COPY --chown=efs:efs services/efs-guardian/docker services/efs-guardian/docker
COPY services/efs-guardian/docker services/efs-guardian/docker
RUN chmod +x services/efs-guardian/docker/*.sh


Expand Down Expand Up @@ -205,7 +203,7 @@ ENV SC_BUILD_TARGET=development \

WORKDIR /devel

RUN chown -R efs:efs "${VIRTUAL_ENV}"
RUN chown -R root:root "${VIRTUAL_ENV}"
matusdrobuliak66 marked this conversation as resolved.
Show resolved Hide resolved

ENTRYPOINT ["/bin/sh", "services/efs-guardian/docker/entrypoint.sh"]
CMD ["/bin/sh", "services/efs-guardian/docker/boot.sh"]
4 changes: 2 additions & 2 deletions services/efs-guardian/docker/entrypoint.sh
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ if stat $DOCKER_MOUNT >/dev/null 2>&1; then
fi

echo "$INFO Starting $* ..."
echo " $EFS_USER_NAME rights : $(id "$EFS_USER_NAME")"
echo " $(whoami) rights : $(id $whoami))"
echo " local dir : $(ls -al)"

exec gosu "$EFS_USER_NAME" "$@"
exec "$@"
matusdrobuliak66 marked this conversation as resolved.
Show resolved Hide resolved
Loading