-
Notifications
You must be signed in to change notification settings - Fork 486
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Add CRD ImageContentPolicy(ICP) to config.openshift.io/v1 to support …
…AllowMirrByTags Update the enhancement to describe the work has been done based on the previous discussions for Epic https://issues.redhat.com/browse/OCPNODE-521 We can continue the discussions from the current design. Signed-off-by: Qi Wang <qiwan@redhat.com>
- Loading branch information
Showing
2 changed files
with
228 additions
and
194 deletions.
There are no files selected for viewing
228 changes: 228 additions & 0 deletions
228
...ements/api-review/add-new-CRD-ImageContentPolicy(ICP)-to-config.openshift.io.md
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,228 @@ | ||
--- | ||
title: add-new-CRD-ImageContentPolicy(ICP)-to-config.openshift.io | ||
authors: | ||
- "@QiWang19" | ||
reviewers: | ||
- TBD | ||
approvers: | ||
- TBD | ||
creation-date: 2021-03-10 | ||
last-updated: 2021-10-08 | ||
status: implementable | ||
--- | ||
|
||
# Add CRD ImageContentPolicy(ICP) to config.openshift.io/v1 to support AllowMirrByTags | ||
|
||
## Release Signoff Checklist | ||
|
||
- [x] Enhancement is `implementable` | ||
- [x] Design details are appropriately documented from clear requirements | ||
- [x] Test plan is defined | ||
- [ ] Operational readiness criteria is defined | ||
- [ ] Graduation criteria for dev preview, tech preview, GA | ||
- [ ] User-facing documentation is created in [openshift-docs](https://github.com/openshift/openshift-docs/) | ||
|
||
## Summary | ||
|
||
Today, the ImageContentSourcePolicy(ICSP) object sets up mirror with `mirror-by-digest-only` property set to true. Requires images be pulled by digest only. This enhancement introduce a new CRD, ImageContentPolicy(ICP) with all fields from ImageContentSourcePolicy(ICSP) and a configurable field `allowMirrorByTags`, so that `mirror-by-digest-only` can be configured by ImageContentPolicy. | ||
|
||
## Motivation | ||
|
||
### Motivation to add support allow mirror by tags | ||
|
||
- Today, the ImageContentSourcePolicy(ICSP) object sets up mirror configuration with `mirror-by-digest-only` property set to `true`, which leads to using mirrors only when images are referenced by digests. However, when working with disconnected environments, sometimes multiple ImageContentSourcePolicies are needed, some of them are used by apps/manifests that don't use digests when pulling the images. | ||
|
||
### Motivation to add a new CRD | ||
|
||
- Current ImageContentSourcePolicy(ICSP) of operator.openshift.io/v1alpha1 was added to operator group after operator.openshift.io/v1. Adding v1alpha1 after v1 was a mistake. We should not continue development on ImageContentSourcePolicy(ICSP) of operator.openshift.io/v1alpha1. | ||
- ImageContentSourcePolicy(ICSP) holds cluster-wide information about how to handle registry mirror rules. It is not about the configuration of an operator. The CRD should be moved to config.openshift.io/v1. | ||
- Current ImageContentSourcePolicy(ICSP) has no kubebuilder annotations to validate the fields of it. We need to add validation for the CRD. Even if we continue with operator.openshift.io, add validation to the field and add ImageContentSourcePolicy(ICSP) to operator.openshift.io/v1 CRDs don't have ratcheting validation yet to cope with that. So a new CRD should be created, and ImageContentSourcePolicy(ICSP) under operator.openshift.io/v1alpha1 will be removed. | ||
|
||
### Goals | ||
|
||
- Enable the user to pull images through ImageContentPolicy configured mirrors by tags. | ||
|
||
### Non-Goals | ||
|
||
- This proposal does not recommend using by-tag references for OpenShift release images. Those should still be referenced by digest, regardless of whether `allowMirrorByTags` is enabled for repositories where the release images are mirrored | ||
|
||
## Proposal | ||
|
||
[openshift/api#874:Create ImageContentPolicy CRD to config/v1 and allowMirrorByTags support](https://github.com/openshift/api/pull/874) PR was merged. New CRD ImageContentPolicy was added to config.openshift.io/v1 PR. | ||
``` | ||
The schema of ImageContentPolicy(ICP) = fields of ImageContentSourcePolicy(ICSP) + allowMirrorByTags | ||
``` | ||
The `allowMirrorByTags` (default false) property will allow for mirroring by-tag images through configured mirrors. | ||
|
||
Code snippet from https://github.com/openshift/api/blob/master/config/v1/types_image_content_policy.go | ||
```go | ||
// ImageContentPolicySpec is the specification of the ImageContentPolicy CRD. | ||
type ImageContentPolicySpec struct { | ||
// repositoryDigestMirrors allows images referenced by image digests in pods to be | ||
// pulled from alternative mirrored repository locations. The image pull specification | ||
// provided to the pod will be compared to the source locations described in RepositoryDigestMirrors | ||
// and the image may be pulled down from any of the mirrors in the list instead of the | ||
// specified repository allowing administrators to choose a potentially faster mirror. | ||
// To pull image from mirrors by tags, should set the "allowMirrorByTags". | ||
// | ||
// Each “source” repository is treated independently; configurations for different “source” | ||
// repositories don’t interact. | ||
// | ||
// If the "mirrors" is not specified, the image will continue to be pulled from the specified | ||
// repository in the pull spec. | ||
// | ||
// When multiple policies are defined for the same “source” repository, the sets of defined | ||
// mirrors will be merged together, preserving the relative order of the mirrors, if possible. | ||
// For example, if policy A has mirrors `a, b, c` and policy B has mirrors `c, d, e`, the | ||
// mirrors will be used in the order `a, b, c, d, e`. If the orders of mirror entries conflict | ||
// (e.g. `a, b` vs. `b, a`) the configuration is not rejected but the resulting order is unspecified. | ||
// +optional | ||
// +listType=map | ||
// +listMapKey=source | ||
RepositoryDigestMirrors []RepositoryDigestMirrors `json:"repositoryDigestMirrors"` | ||
} | ||
|
||
// RepositoryDigestMirrors holds cluster-wide information about how to handle mirrors in the registries config. | ||
type RepositoryDigestMirrors struct { | ||
// source is the repository that users refer to, e.g. in image pull specifications. | ||
// +required | ||
// +kubebuilder:validation:Required | ||
// +kubebuilder:validation:Pattern=`^(([a-zA-Z]|[a-zA-Z][a-zA-Z0-9\-]*[a-zA-Z0-9])\.)*([A-Za-z]|[A-Za-z][A-Za-z0-9\-]*[A-Za-z0-9])(:[0-9]+)?(\/[^\/:\n]+)*(\/[^\/:\n]+((:[^\/:\n]+)|(@[^\n]+)))?$` | ||
Source string `json:"source"` | ||
// allowMirrorByTags if true, the mirrors can be used to pull the images that are referenced by their tags. Default is false, the mirrors only work when pulling the images that are referenced by their digests. | ||
// Pulling images by tag can potentially yield different images, depending on which endpoint | ||
// we pull from. Forcing digest-pulls for mirrors avoids that issue. | ||
// +optional | ||
AllowMirrorByTags bool `json:"allowMirrorByTags,omitempty"` | ||
// mirrors is zero or more repositories that may also contain the same images. | ||
// If the "mirrors" is not specified, the image will continue to be pulled from the specified | ||
// repository in the pull spec. No mirror will be configured. | ||
// The order of mirrors in this list is treated as the user's desired priority, while source | ||
// is by default considered lower priority than all mirrors. Other cluster configuration, | ||
// including (but not limited to) other repositoryDigestMirrors objects, | ||
// may impact the exact order mirrors are contacted in, or some mirrors may be contacted | ||
// in parallel, so this should be considered a preference rather than a guarantee of ordering. | ||
// +optional | ||
// +listType=set | ||
Mirrors []Mirror `json:"mirrors,omitempty"` | ||
} | ||
|
||
// +kubebuilder:validation:Pattern=`^(([a-zA-Z]|[a-zA-Z][a-zA-Z0-9\-]*[a-zA-Z0-9])\.)*([A-Za-z]|[A-Za-z][A-Za-z0-9\-]*[A-Za-z0-9])(:[0-9]+)?(\/[^\/:\n]+)*(\/[^\/:\n]+((:[^\/:\n]+)|(@[^\n]+)))?$` | ||
type Mirror string | ||
``` | ||
|
||
ImageContentPolicy CRD will now watch for mirrors configured through spec `repositoryMirrors` to not require digest reference. | ||
|
||
An example ImageContentPolicy file will look like: | ||
```yaml | ||
apiVersion: config.openshift.io/v1 | ||
kind: ImageContentPolicy | ||
metadata: | ||
name: ubi8repo | ||
spec: | ||
repositoryMirrors: | ||
- mirrors: | ||
- example.io/example/ubi-minimal | ||
source: registry.access.redhat.com/ubi8/ubi-minimal | ||
allowMirrorByTags: true | ||
``` | ||
The ImageContentPolicy file will create a drop-in file at `/etc/containers/registries.conf` that looks like: | ||
|
||
```toml | ||
unqualified-search-registries = ["registry.access.redhat.com", "docker.io"] | ||
[[registry]] | ||
location = "registry.access.redhat.com/ubi8/" | ||
insecure = false | ||
blocked = false | ||
# No mirror-by-digest-only = true configured | ||
prefix = "" | ||
[[registry.mirror]] | ||
location = "example.io/example/ubi8-minimal" | ||
insecure = false | ||
``` | ||
|
||
### User Stories | ||
|
||
#### As a user, I would like to pull images from mirror using tag, without digests reference | ||
|
||
The user need to define multiple ImageContentPolicies, used by apps/manifests that don't use digests to pull | ||
the images when working with disconnected environments or pulling from registries that act as transparent pull-through proxy cache. | ||
|
||
The user can pull image without digest by configuring `allowMirrorByTags: true` in the ImageContentPolicy file. | ||
And create the ImageContentPolicy project. Once this is done, the images can be pulled from the mirrors without the digest referenced. | ||
|
||
### Implementation Details/Notes/Constraints [optional] | ||
|
||
Implementing this enhancement requires changes in: | ||
- [openshift/api/config/v1](https://github.com/openshift/api/tree/master/config/v1): definition the schema and API of the ImageContentPolicy CRD. | ||
- [openshift/runtime-utils/pkg/registries](https://github.com/openshift/runtime-utils/tree/master/pkg/registries): helper functions to edit `mirror-by-tags-only` in registries.conf. | ||
- [openshift/machine-config-operator](https://github.com/openshift/machine-config-operator): MCO needs watch for the ImageContentPolicy. | ||
|
||
#### Constraints Need Discussion | ||
|
||
1. The impact on ImageStream and `oc` command is undefined. | ||
2. Design of the rejction of conflicting `allowMirrorByTags` values for the same `source` of multiple indenpendant CRs. | ||
3. The merge order of mirrors for the same source. | ||
Current order is preserving the relative order of the mirrors, if possible. | ||
For example, if policy A has mirrors `a, b, c` and policy B has mirrors `c, d, e`, the | ||
mirrors will be used in the order `a, b, c, d, e`. If the orders of mirror entries conflict | ||
(e.g. `a, b` vs. `b, a`) the configuration is not rejected but the resulting order is unspecified. | ||
|
||
|
||
|
||
### Risks and Mitigations | ||
|
||
Pulling images from mirror registries without the digest specifications could lead to returning different image version from different registry if the image tag mapping is out of sync. But the OpenShift core required image using digests to avoid different versions won't consume this feature at all, so it is not exposed to the risks that anyone who actually uses the feature will be exposed to. | ||
|
||
## Design Details | ||
|
||
### Test Plan | ||
|
||
Update the tests that are currently in the MCO to verify that `mirror-by-digest-only` not have been set when spec `allowMirrorByTags: true` is specified in the ImageContentPolicy. | ||
|
||
### Graduation Criteria | ||
|
||
GA in openshift will be looked at to | ||
determine graduation. | ||
|
||
#### Dev Preview -> Tech Preview | ||
|
||
- Ability to utilize the enhancement end to end | ||
- End user documentation, relative API stability | ||
- Sufficient test coverage | ||
- Gather feedback from users rather than just developers | ||
- Enumerate service level indicators (SLIs), expose SLIs as metrics | ||
- Write symptoms-based alerts for the component(s) | ||
|
||
#### Tech Preview -> GA | ||
|
||
- More testing (upgrade, downgrade, scale) | ||
- Sufficient time for feedback | ||
- Available by default | ||
- Backhaul SLI telemetry | ||
- Document SLOs for the component | ||
- Conduct load testing | ||
|
||
#### Removing a deprecated feature | ||
|
||
- The CRD ImageContentSourcePolicy(ICSP) will be removed. A Jira card [OCPNODE-717](https://issues.redhat.com/browse/OCPNODE-717) was created to record the upgrade of ImageContenPolicy(ICP) . The [repositories](https://docs.google.com/document/d/11FJPpIYAQLj5EcYiJtbi_bNkAcJa2hCLV63WvoDsrcQ/edit) currently rely on operator/v1alpha1 ImageContentSourcePolicy(ICSP) will be migrated to config/v1 ImageContentPolicy. | ||
|
||
### Upgrade / Downgrade Strategy | ||
|
||
Upgrades should not be affect. But the downgrade will be affected. | ||
Users upgraded and configured the `ImageContentPolicy` spec will presumably have their CRI-O configurations clobbered and break their tag-mirroring-dependent workflows after they downgrade to a version that lacks support for the `ImageContentPolicy`. | ||
|
||
### Version Skew Strategy | ||
|
||
Upgrade skew will not impact this feature. The MCO does not require skew check. CRI-O with n-2 OpenShift skew will still be able to handle the new property. | ||
|
||
## Implementation History | ||
|
||
- [openshift/api#874: Create ImageContentPolicy CRD to config/v1 and allowMirrorByTags support](https://github.com/openshift/api/pull/874) PR was merged. | ||
## Drawbacks | ||
|
||
## Alternatives | ||
|
||
## Infrastructure Needed [optional] |
Oops, something went wrong.