diff --git a/changelog.d/20250204_145210_athornton_DM_48682_config.md b/changelog.d/20250204_145210_athornton_DM_48682_config.md new file mode 100644 index 000000000..8263ca5b7 --- /dev/null +++ b/changelog.d/20250204_145210_athornton_DM_48682_config.md @@ -0,0 +1,4 @@ + +### Backwards-incompatible changes + +- Changed `num_*` in config to image policy object. diff --git a/controller/src/controller/config.py b/controller/src/controller/config.py index b32ea9504..f307fe902 100644 --- a/controller/src/controller/config.py +++ b/controller/src/controller/config.py @@ -34,17 +34,20 @@ RESERVED_ENV, RESERVED_PATHS, ) +from .models.domain.imagepolicy import ( + RSPImagePolicy, +) from .models.domain.kubernetes import ( Affinity, PullPolicy, Toleration, VolumeAccessMode, ) +from .models.domain.menu import ImageDisplayPolicy, SpawnerMenuOptions from .models.v1.lab import LabResources, LabSize, ResourceQuantity from .models.v1.prepuller import ( DockerSourceOptions, GARSourceOptions, - PrepullerOptions, ) from .units import memory_to_bytes @@ -64,6 +67,7 @@ "NFSVolumeSource", "PVCVolumeResources", "PVCVolumeSource", + "SpawnerMenuConfig", "TmpSource", "UserHomeDirectorySchema", "VolumeConfig", @@ -505,7 +509,8 @@ class GARSourceConfig(GARSourceOptions): """Configuration for a Google Artifact Registry source. This is identical to the API model used to return the prepuller - configuration to an API client except that camel-case aliases are enabled. + configuration to an API client except that camel-case aliases are + enabled. """ model_config = ConfigDict( @@ -513,11 +518,40 @@ class GARSourceConfig(GARSourceOptions): ) -class PrepullerConfig(PrepullerOptions): - """Configuration for the prepuller. +class ImageDisplayPolicyConfig(ImageDisplayPolicy): + """Configuration for ImageDisplayPolicy. - This is identical to the API model used to return the prepuller - configuration to an API client except that camel-case aliases are enabled. + This is identical to the model used for image display policy, except + that camel-case aliases are enabled. + """ + + model_config = ConfigDict( + alias_generator=to_camel, extra="forbid", populate_by_name=True + ) + + main: RSPImagePolicyConfig | None = None + dropdown: RSPImagePolicyConfig | None = None + + +class RSPImagePolicyConfig(RSPImagePolicy): + """Configuration for RSPImagePolicy. + + This is identical to the model used for RSP image policy, except + that camel-case aliases are enabled. + """ + + model_config = ConfigDict( + alias_generator=to_camel, extra="forbid", populate_by_name=True + ) + + +class SpawnerMenuConfig(SpawnerMenuOptions): + """Configuration for how images are presented on the spawner page. + + It controls both the main menu and the dropdown menu. + + This is identical to the model used for menu control, except that + camel-case aliases are enabled. """ model_config = ConfigDict( @@ -525,6 +559,7 @@ class PrepullerConfig(PrepullerOptions): ) source: DockerSourceConfig | GARSourceConfig + display_policy: ImageDisplayPolicyConfig class LabSizeDefinition(BaseModel): @@ -1202,13 +1237,13 @@ class Config(BaseSettings): ] = DisabledFileserverConfig() images: Annotated[ - PrepullerConfig, + SpawnerMenuConfig, Field( title="Available lab images", description=( "Configuration for which images to prepull and which images to" - " display in the spawner menu for users to choose from when" - " spawning labs" + " display in the spawner menu and dropdown for users to choose" + " from when spawning labs" ), ), ] diff --git a/controller/src/controller/exceptions.py b/controller/src/controller/exceptions.py index 14d5eec5e..1be183c65 100644 --- a/controller/src/controller/exceptions.py +++ b/controller/src/controller/exceptions.py @@ -36,6 +36,7 @@ "KubernetesError", "LabDeletionError", "LabExistsError", + "MissingImageCountError", "MissingObjectError", "MissingSecretError", "NoOperationError", @@ -437,6 +438,14 @@ class LabDeletionError(SlackException): """ +class MissingImageCountError(SlackException): + """Image count is not specified. + + When constructing the spawner menu, we need to know how many daily, + weekly, and release images to display. + """ + + class MissingObjectError(SlackException): """An expected Kubernetes object is missing. diff --git a/controller/src/controller/factory.py b/controller/src/controller/factory.py index f4b658cc6..715dbfe41 100644 --- a/controller/src/controller/factory.py +++ b/controller/src/controller/factory.py @@ -18,7 +18,10 @@ from .config import Config from .events import LabEvents from .exceptions import NotConfiguredError -from .models.v1.prepuller import DockerSourceOptions, GARSourceOptions +from .models.v1.prepuller import ( + DockerSourceOptions, + GARSourceOptions, +) from .services.builder.fileserver import FileserverBuilder from .services.builder.lab import LabBuilder from .services.builder.prepuller import PrepullerBuilder @@ -145,7 +148,7 @@ async def from_config(cls, config: Config) -> Self: metadata_storage = MetadataStorage(config.metadata_path) image_service = ImageService( - config=config.images, + config=config.images.to_prepuller_options(), node_selector=config.lab.node_selector, tolerations=config.lab.tolerations, source=source, diff --git a/controller/src/controller/models/domain/imagepolicy.py b/controller/src/controller/models/domain/imagepolicy.py new file mode 100644 index 000000000..51784d76f --- /dev/null +++ b/controller/src/controller/models/domain/imagepolicy.py @@ -0,0 +1,106 @@ +"""Models for image display policy.""" + +import datetime +from typing import Annotated, Any + +from pydantic import BaseModel, BeforeValidator, Field, model_validator +from safir.pydantic import validate_exactly_one_of + + +def _empty_str_is_none(inp: Any) -> Any: + if isinstance(inp, str) and inp == "": + return None + return inp + + +class IndividualImageClassPolicy(BaseModel): + """Policy for images to display within a given class. + + The policy has both a 'number' and an 'age' field. + + 'number' means: display that many of whatever image class this is + attached to. `-1` or `None` are interpreted as "do not filter + (i.e. show all of this image class)" and `0` means "display no + images of this class." This has historically been the only filter + option. + + 'age' means: display any items of the class whose age is the + specified age or less. This age is a duration, specified by a + string as accepted by Safir's HumanTimeDelta. The empty string + means "do not filter this class at all." + + Exactly one of these must be specified. + """ + + age: Annotated[ + datetime.timedelta | None, + Field( + BeforeValidator(_empty_str_is_none), + title="Age", + description="Maximum age of image to retain.", + ), + ] = None + + number: Annotated[ + int | None, + Field( + BeforeValidator(_empty_str_is_none), + title="Number", + description="Number of images to retain.", + ), + ] = None + + _validate_options = model_validator(mode="after")( + validate_exactly_one_of("number", "age") + ) + + +class RSPImagePolicy(BaseModel): + """Aliases are never filtered. Default for everything else is "do not + filter". + """ + + release: Annotated[ + IndividualImageClassPolicy | None, + Field(title="Release", description="Policy for releases to display."), + ] = None + + weekly: Annotated[ + IndividualImageClassPolicy | None, + Field( + title="Weekly", description="Policy for weekly builds to display." + ), + ] = None + + daily: Annotated[ + IndividualImageClassPolicy | None, + Field( + title="Daily", description="Policy for daily builds to display." + ), + ] = None + + release_candidate: Annotated[ + IndividualImageClassPolicy | None, + Field( + title="Release Candidate", + description="Policy for release candidate builds to display.", + ), + ] = None + + experimental: Annotated[ + IndividualImageClassPolicy | None, + Field( + title="Experimental", + description="Policy for experimental builds to display.", + ), + ] = None + + unknown: Annotated[ + IndividualImageClassPolicy | None, + Field( + title="Unknown", + description=( + "Policy for builds without parseable RSP tags to display." + ), + ), + ] = None diff --git a/controller/src/controller/models/domain/menu.py b/controller/src/controller/models/domain/menu.py new file mode 100644 index 000000000..e2ecf7290 --- /dev/null +++ b/controller/src/controller/models/domain/menu.py @@ -0,0 +1,139 @@ +"""Model for spawner menu.""" + +from typing import Annotated + +from pydantic import BaseModel, Field + +from ...exceptions import MissingImageCountError +from ..v1.prepuller import ( + DockerSourceOptions, + GARSourceOptions, + PrepullerOptions, +) +from .imagepolicy import RSPImagePolicy + +__all__ = ["ImageDisplayPolicy", "SpawnerMenuOptions"] + + +class ImageDisplayPolicy(BaseModel): + """Holds image display policies for spawner main and dropdown menus.""" + + main: Annotated[ + RSPImagePolicy | None, + Field(title="Image display policy for spawner main menu"), + ] = None + + dropdown: Annotated[ + RSPImagePolicy | None, + Field(title="Image display policy for spawner dropdown menu"), + ] = None + + +class SpawnerMenuOptions(BaseModel): + """Options needed to construct the spawner menu. + + This class and PrepullerOptions are largely the same. If we prefer we + could change the API and get rid of PrepullerOptions. + """ + + source: Annotated[ + DockerSourceOptions | GARSourceOptions, Field(title="Source of images") + ] + + display_policy: Annotated[ + ImageDisplayPolicy | None, + Field(title="Display policy for spawner images"), + ] = None + + recommended_tag: Annotated[ + str, + Field( + title="Tag of recommended image", + description=( + "This image will be shown first on the menu as the default" + " choice." + ), + examples=["recommended"], + ), + ] = "recommended" + + cycle: Annotated[ + int | None, + Field( + title="Limit to this cycle number (XML schema version)", + description=( + "Telescope and Site images contain software implementing a" + " specific XML schema version, and it is not safe to use" + " software using a different XML schema version. If this is" + " set, only images with a matching cycle will be shown in the" + " spawner menu." + ), + examples=[27], + ), + ] = None + + pin: Annotated[ + list[str], + Field( + title="List of image tags to prepull and pin to the menu", + description=( + "Forces images to be cached and pinned to the menu even when" + " they would not normally be prepulled (not recommended or" + " within the latest dailies, weeklies, or releases). This can" + " be used to add additional images to the menu or to force" + " resolution of the image underlying the recommended tag when" + " Docker is used as the image source so that we can give it a" + " proper display name." + ), + examples=[["d_2077_10_23"]], + ), + ] = [] + + alias_tags: Annotated[ + list[str], + Field( + title="Additional alias tags", + description=( + "These tags will automatically be recognized as alias tags" + " rather than unknown tags, which results in different sorting" + " and better human-readable descriptions." + ), + examples=[["recommended_cycle0027"]], + ), + ] = [] + + def to_prepuller_options(self) -> PrepullerOptions: + """Construct PrepullerOptions from SpawnerMenuOptions. + + The underlying presumption, that may not be obvious at first glance, + is that the main menu on the Spawner Menu contains definitionally + exactly those images that should be prepulled. + """ + if self.display_policy is None or self.display_policy.main is None: + raise MissingImageCountError( + "Display policy for 'main' in spawner menu options must" + " be defined" + ) + pol = self.display_policy.main + if ( + pol.release is None + or pol.release.number is None + or pol.weekly is None + or pol.weekly.number is None + or pol.daily is None + or pol.daily.number is None + ): + raise MissingImageCountError( + "All of 'daily', 'weekly', and 'release' must be defined and" + " numeric in display policy to generate prepuller options" + ) + return PrepullerOptions( + source=self.source, + recommended_tag=self.recommended_tag, + cycle=self.cycle, + pin=self.pin, + alias_tags=self.alias_tags, + num_releases=pol.release.number, + num_weeklies=pol.weekly.number, + num_dailies=pol.daily.number, + ) diff --git a/controller/src/controller/models/v1/prepuller.py b/controller/src/controller/models/v1/prepuller.py index 50e7ad7c8..c3dd1390d 100644 --- a/controller/src/controller/models/v1/prepuller.py +++ b/controller/src/controller/models/v1/prepuller.py @@ -148,11 +148,8 @@ def path(self) -> str: class PrepullerOptions(BaseModel): """Options for the prepuller. - The information here comes from the YAML configuration for the Nublado - controller and is a component of the model returned by the - ``/spawner/v1/prepulls`` route. The model for the YAML configuration also - enables camel-case aliases, but those are not enabled here since we want - the API to return snake-case. + The information here is a component of the model returned by the + ``/spawner/v1/prepulls`` route. """ source: Annotated[ diff --git a/controller/tests/data/changed-path/input/config.yaml b/controller/tests/data/changed-path/input/config.yaml index 0e798153d..d3b5b424d 100644 --- a/controller/tests/data/changed-path/input/config.yaml +++ b/controller/tests/data/changed-path/input/config.yaml @@ -23,3 +23,11 @@ images: type: docker registry: lighthouse.ceres repository: library/sketchbook + displayPolicy: + main: + release: + number: 1 + weekly: + number: 2 + daily: + number: 3 diff --git a/controller/tests/data/cycle/input/config.yaml b/controller/tests/data/cycle/input/config.yaml index 5c9abad11..51da2ef43 100644 --- a/controller/tests/data/cycle/input/config.yaml +++ b/controller/tests/data/cycle/input/config.yaml @@ -12,9 +12,14 @@ images: registry: lighthouse.ceres repository: library/sketchbook recommendedTag: recommended_c0050 - numReleases: 0 - numWeeklies: 3 - numDailies: 0 cycle: 50 aliasTags: - latest_c0050 + displayPolicy: + main: + release: + number: 0 + weekly: + number: 3 + daily: + number: 0 diff --git a/controller/tests/data/extra-annotations/input/config.yaml b/controller/tests/data/extra-annotations/input/config.yaml index 5351e15d1..781f10ad3 100644 --- a/controller/tests/data/extra-annotations/input/config.yaml +++ b/controller/tests/data/extra-annotations/input/config.yaml @@ -23,3 +23,11 @@ images: type: docker registry: lighthouse.ceres repository: library/sketchbook + displayPolicy: + main: + release: + number: 1 + weekly: + number: 2 + daily: + number: 3 diff --git a/controller/tests/data/fileserver/input/config.yaml b/controller/tests/data/fileserver/input/config.yaml index b75c642ba..a08be90fc 100644 --- a/controller/tests/data/fileserver/input/config.yaml +++ b/controller/tests/data/fileserver/input/config.yaml @@ -45,6 +45,14 @@ images: type: docker registry: lighthouse.ceres repository: library/sketchbook + displayPolicy: + main: + release: + number: 1 + weekly: + number: 2 + daily: + number: 3 fileserver: enabled: true affinity: diff --git a/controller/tests/data/gar-cycle/input/config.yaml b/controller/tests/data/gar-cycle/input/config.yaml index 2cc9b7f67..4639b92dc 100644 --- a/controller/tests/data/gar-cycle/input/config.yaml +++ b/controller/tests/data/gar-cycle/input/config.yaml @@ -14,7 +14,12 @@ images: repository: library image: sketchbook recommendedTag: recommended_c0050 - numReleases: 0 - numWeeklies: 3 - numDailies: 0 cycle: 50 + displayPolicy: + main: + release: + number: 0 + weekly: + number: 3 + daily: + number: 0 diff --git a/controller/tests/data/gar/input/config.yaml b/controller/tests/data/gar/input/config.yaml index d71d25059..bdecaa85e 100644 --- a/controller/tests/data/gar/input/config.yaml +++ b/controller/tests/data/gar/input/config.yaml @@ -14,6 +14,11 @@ images: repository: sciplat image: sciplat-lab recommendedTag: recommended - numReleases: 1 - numWeeklies: 2 - numDailies: 3 + displayPolicy: + main: + release: + number: 1 + weekly: + number: 2 + daily: + number: 3 diff --git a/controller/tests/data/homedir-schema/input/config.yaml b/controller/tests/data/homedir-schema/input/config.yaml index f7b5aed01..66dff9628 100644 --- a/controller/tests/data/homedir-schema/input/config.yaml +++ b/controller/tests/data/homedir-schema/input/config.yaml @@ -26,3 +26,11 @@ images: type: docker registry: lighthouse.ceres repository: library/sketchbook + displayPolicy: + main: + release: + number: 1 + weekly: + number: 2 + daily: + number: 3 diff --git a/controller/tests/data/prepuller/input/config.yaml b/controller/tests/data/prepuller/input/config.yaml index 36a5afee2..5c67261db 100644 --- a/controller/tests/data/prepuller/input/config.yaml +++ b/controller/tests/data/prepuller/input/config.yaml @@ -17,3 +17,11 @@ images: type: docker registry: lighthouse.ceres repository: library/sketchbook + displayPolicy: + main: + release: + number: 1 + weekly: + number: 2 + daily: + number: 3 diff --git a/controller/tests/data/standard/input/config.yaml b/controller/tests/data/standard/input/config.yaml index 443cd9bad..fe9d07ff6 100644 --- a/controller/tests/data/standard/input/config.yaml +++ b/controller/tests/data/standard/input/config.yaml @@ -231,6 +231,11 @@ images: registry: lighthouse.ceres repository: library/sketchbook recommendedTag: recommended - numReleases: 1 - numWeeklies: 2 - numDailies: 3 + displayPolicy: + main: + release: + number: 1 + weekly: + number: 2 + daily: + number: 3 diff --git a/controller/tests/data/tmp-disk/input/config.yaml b/controller/tests/data/tmp-disk/input/config.yaml index d7e6c92db..25afd63cc 100644 --- a/controller/tests/data/tmp-disk/input/config.yaml +++ b/controller/tests/data/tmp-disk/input/config.yaml @@ -21,3 +21,11 @@ images: type: docker registry: lighthouse.ceres repository: library/sketchbook + displayPolicy: + main: + release: + number: 1 + weekly: + number: 2 + daily: + number: 3 diff --git a/docs/dev/api/controller.rst b/docs/dev/api/controller.rst index 19a4a76e1..d8a75dcc3 100644 --- a/docs/dev/api/controller.rst +++ b/docs/dev/api/controller.rst @@ -48,12 +48,18 @@ This documentation therefore exists only to assist developers and code analysis .. automodapi:: controller.models.domain.image :include-all-objects: +.. automodapi:: controller.models.domain.imagepolicy + :include-all-objects: + .. automodapi:: controller.models.domain.kubernetes :include-all-objects: .. automodapi:: controller.models.domain.lab :include-all-objects: +.. automodapi:: controller.models.domain.menu + :include-all-objects: + .. automodapi:: controller.models.domain.rspimage :include-all-objects: