Skip to content

Commit

Permalink
Merge branch 'master' into fix/po-center-bad-setting
Browse files Browse the repository at this point in the history
  • Loading branch information
pcrespov authored Oct 7, 2024
2 parents 3531d07 + d4e5471 commit d7cf591
Show file tree
Hide file tree
Showing 9 changed files with 88 additions and 34 deletions.
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",
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(
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}"

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 "$@"

0 comments on commit d7cf591

Please sign in to comment.