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

MCO-1521: Promote PinnedImageSet to GA #2198

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
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
1 change: 0 additions & 1 deletion hack/update-payload-crds.sh
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ crd_globs="\
operator/v1/zz_generated.crd-manifests/0000_50_openshift-controller-manager_02_openshiftcontrollermanagers*.crd.yaml
machineconfiguration/v1/zz_generated.crd-manifests/*.crd.yaml
machineconfiguration/v1alpha1/zz_generated.crd-manifests/0000_80_machine-config_01_machineconfignodes*.crd.yaml
machineconfiguration/v1alpha1/zz_generated.crd-manifests/0000_80_machine-config_01_pinnedimagesets*.crd.yaml
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is telling the payload to remove the v1alpha1 CRDs and install the v1 CRDs, which I think will break techpreview since the MCO wouldn't know how to process these in the meantime?

i.e. I think we would need to create the v1 API through this PR, and then still install alpha CRDs for now, then move the MCO to process v1 API, and simultaneously have the CRDs switch to v1 here as a second step.

operator/v1/zz_generated.crd-manifests/0000_80_machine-config_01_machineconfigurations*.crd.yaml
config/v1alpha1/zz_generated.crd-manifests/0000_10_config-operator_01_clusterimagepolicies*.crd.yaml
config/v1alpha1/zz_generated.crd-manifests/0000_10_config-operator_01_imagepolicies*.crd.yaml
Expand Down
2 changes: 2 additions & 0 deletions machineconfiguration/v1/register.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ func addKnownTypes(scheme *runtime.Scheme) error {
&MachineOSConfigList{},
&MachineOSBuild{},
&MachineOSBuildList{},
&PinnedImageSet{},
&PinnedImageSetList{},
)

metav1.AddToGroupVersion(scheme, GroupVersion)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
apiVersion: apiextensions.k8s.io/v1 # Hack because controller-gen complains if we don't have this
name: "PinnedImageSet"
crdName: pinnedimagesets.machineconfiguration.openshift.io
featureGate: PinnedImages
tests:
onCreate:
- name: Should be able to create a minimal PinnedImageSet
initial: |
apiVersion: machineconfiguration.openshift.io/v1
kind: PinnedImageSet
metadata:
name: foobar
labels:
machineconfiguration.openshift.io/role: "master"
spec:
pinnedImages:
- name: registry.example.com/custom-os-image@sha256:86d26e7ebcccd6f07a75db5b1e56283b25c2ee1c6a755d6ffc5a4d59beb9c504
expected: |
apiVersion: machineconfiguration.openshift.io/v1
kind: PinnedImageSet
metadata:
name: foobar
labels:
machineconfiguration.openshift.io/role: "master"
spec:
pinnedImages:
- name: registry.example.com/custom-os-image@sha256:86d26e7ebcccd6f07a75db5b1e56283b25c2ee1c6a755d6ffc5a4d59beb9c504
- name: Should be able to create a PinnedImageSet with the PinnedImageRef name containing a port
initial: |
apiVersion: machineconfiguration.openshift.io/v1
kind: PinnedImageSet
metadata:
name: foobar
labels:
machineconfiguration.openshift.io/role: "master"
spec:
pinnedImages:
- name: registry.example.com:5000/custom-os-image@sha256:86d26e7ebcccd6f07a75db5b1e56283b25c2ee1c6a755d6ffc5a4d59beb9c504
expected: |
apiVersion: machineconfiguration.openshift.io/v1
kind: PinnedImageSet
metadata:
name: foobar
labels:
machineconfiguration.openshift.io/role: "master"
spec:
pinnedImages:
- name: registry.example.com:5000/custom-os-image@sha256:86d26e7ebcccd6f07a75db5b1e56283b25c2ee1c6a755d6ffc5a4d59beb9c504
- name: Should be able to create a PinnedImageSet with the PinnedImageRef name containing a namespace
initial: |
apiVersion: machineconfiguration.openshift.io/v1
kind: PinnedImageSet
metadata:
name: foobar
labels:
machineconfiguration.openshift.io/role: "master"
spec:
pinnedImages:
- name: registry.example.com/my-namespace/custom-os-image@sha256:86d26e7ebcccd6f07a75db5b1e56283b25c2ee1c6a755d6ffc5a4d59beb9c504
expected: |
apiVersion: machineconfiguration.openshift.io/v1
kind: PinnedImageSet
metadata:
name: foobar
labels:
machineconfiguration.openshift.io/role: "master"
spec:
pinnedImages:
- name: registry.example.com/my-namespace/custom-os-image@sha256:86d26e7ebcccd6f07a75db5b1e56283b25c2ee1c6a755d6ffc5a4d59beb9c504
- name: Fail on invalid PinnedImageRef name
initial: |
apiVersion: machineconfiguration.openshift.io/v1
kind: PinnedImageSet
metadata:
name: foobar
labels:
machineconfiguration.openshift.io/role: "master"
spec:
pinnedImages:
- name: foo.bar
expectedError: "spec.pinnedImages[0].name: Invalid value: \"string\": the OCI Image reference must end with a valid '@sha256:<digest>' suffix, where '<digest>' is 64 characters long"
105 changes: 105 additions & 0 deletions machineconfiguration/v1/types_pinnedimageset.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
package v1

import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

// +genclient
// +genclient:nonNamespaced
// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
// +kubebuilder:object:root=true
// +kubebuilder:resource:path=pinnedimagesets,scope=Cluster
// +kubebuilder:subresource:status
// +openshift:api-approved.openshift.io=https://github.com/openshift/api/pull/2198
// +openshift:file-pattern=cvoRunLevel=0000_80,operatorName=machine-config,operatorOrdering=01
// +openshift:enable:FeatureGate=PinnedImages
// +kubebuilder:metadata:labels=openshift.io/operator-managed=

// PinnedImageSet describes a set of images that should be pinned by CRI-O and
// pulled to the nodes which are members of the declared MachineConfigPools.
//
// Compatibility level 1: Stable within a major release for a minimum of 12 months or 3 minor releases (whichever is longer).
// +openshift:compatibility-gen:level=1
type PinnedImageSet struct {
metav1.TypeMeta `json:",inline"`

// metadata is the standard object metadata.
// +optional
metav1.ObjectMeta `json:"metadata,omitempty"`

// spec describes the configuration of this pinned image set.
// +required
Spec PinnedImageSetSpec `json:"spec"`

// status describes the last observed state of this pinned image set.
// +optional
Status PinnedImageSetStatus `json:"status,omitempty"`
}

// PinnedImageSetStatus describes the current state of a PinnedImageSet.
type PinnedImageSetStatus struct {
// conditions represent the observations of a pinned image set's current state.
// +patchMergeKey=type
// +patchStrategy=merge
// +kubebuilder:validation:MaxItems=10
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doing a make update locally causes this to regen, so I think this might be missing from the generated CRs

// +listType=map
// +listMapKey=type
// +optional
Conditions []metav1.Condition `json:"conditions,omitempty" patchStrategy:"merge" patchMergeKey:"type"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thinking through this a bit more, since we can have (500?) pinned images, should we have a status corresponding to each pinned image instead of one general conditions? So multiple failures can be reflected properly

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, maybe not, since the PinnedImageSet would then have the reflect error for every node trying to update to it, will think through this a bit more via the MCN PR

}

// PinnedImageSetSpec defines the desired state of a PinnedImageSet.
type PinnedImageSetSpec struct {
// pinnedImages is a list of OCI Image referenced by digest that should be
// pinned and pre-loaded by the nodes of a MachineConfigPool.
// Translates into a new file inside the /etc/crio/crio.conf.d directory
// with content similar to this:
//
// pinned_images = [
// "quay.io/openshift-release-dev/ocp-release@sha256:...",
// "quay.io/openshift-release-dev/ocp-v4.0-art-dev@sha256:...",
// "quay.io/openshift-release-dev/ocp-v4.0-art-dev@sha256:...",
// ...
// ]
//
// These image references should all be by digest, tags aren't allowed.
// +required
// +kubebuilder:validation:MinItems=1
// +kubebuilder:validation:MaxItems=500
// +listType=map
// +listMapKey=name
PinnedImages []PinnedImageRef `json:"pinnedImages"`
}

type PinnedImageRef struct {
// name is an OCI Image referenced by digest.
//
// The format of the image ref is:
// host[:port][/namespace]/name@sha256:<digest>
// +required
// +kubebuilder:validation:MinLength=1
// +kubebuilder:validation:MaxLength=447
// +kubebuilder:validation:XValidation:rule=`self.split('@').size() == 2 && self.split('@')[1].matches('^sha256:[a-f0-9]{64}$')`,message="the OCI Image reference must end with a valid '@sha256:<digest>' suffix, where '<digest>' is 64 characters long"
// +kubebuilder:validation:XValidation:rule=`self.split('@')[0].matches('^([a-zA-Z0-9-]+\\.)+[a-zA-Z0-9-]+(:[0-9]{2,5})?/([a-zA-Z0-9-_]{0,61}/)?[a-zA-Z0-9-_.]*?$')`,message="the OCI Image name should follow the host[:port][/namespace]/name format, resembling a valid URL without the scheme"
Name string `json:"name"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: within our MachineOSConfig object, we have a type (with validation) called ImageDigestFormat now. I wonder if it's worth it to consolidate them and have type validation the same across all MCO references for digest based images

}

// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object

// PinnedImageSetList is a list of PinnedImageSet resources
//
// Compatibility level 1: Stable within a major release for a minimum of 12 months or 3 minor releases (whichever is longer).
// +openshift:compatibility-gen:level=1
type PinnedImageSetList struct {
metav1.TypeMeta `json:",inline"`

// metadata is the standard list metadata.
// More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#metadata
// +optional
metav1.ListMeta `json:"metadata,omitempty"`

// items contains a collection of PinnedImageSet resources.
// +kubebuilder:validation:MaxItems=500
// +optional
Items []PinnedImageSet `json:"items"`
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,168 @@
apiVersion: apiextensions.k8s.io/v1
kind: CustomResourceDefinition
metadata:
annotations:
api-approved.openshift.io: https://github.com/openshift/api/pull/2198
api.openshift.io/merged-by-featuregates: "true"
include.release.openshift.io/ibm-cloud-managed: "true"
include.release.openshift.io/self-managed-high-availability: "true"
release.openshift.io/feature-set: CustomNoUpgrade
labels:
openshift.io/operator-managed: ""
name: pinnedimagesets.machineconfiguration.openshift.io
spec:
group: machineconfiguration.openshift.io
names:
kind: PinnedImageSet
listKind: PinnedImageSetList
plural: pinnedimagesets
singular: pinnedimageset
scope: Cluster
versions:
- name: v1
schema:
openAPIV3Schema:
description: |-
PinnedImageSet describes a set of images that should be pinned by CRI-O and
pulled to the nodes which are members of the declared MachineConfigPools.

Compatibility level 1: Stable within a major release for a minimum of 12 months or 3 minor releases (whichever is longer).
properties:
apiVersion:
description: |-
APIVersion defines the versioned schema of this representation of an object.
Servers should convert recognized schemas to the latest internal value, and
may reject unrecognized values.
More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources
type: string
kind:
description: |-
Kind is a string value representing the REST resource this object represents.
Servers may infer this from the endpoint the client submits requests to.
Cannot be updated.
In CamelCase.
More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds
type: string
metadata:
type: object
spec:
description: spec describes the configuration of this pinned image set.
properties:
pinnedImages:
description: |-
pinnedImages is a list of OCI Image referenced by digest that should be
pinned and pre-loaded by the nodes of a MachineConfigPool.
Translates into a new file inside the /etc/crio/crio.conf.d directory
with content similar to this:

pinned_images = [
"quay.io/openshift-release-dev/ocp-release@sha256:...",
"quay.io/openshift-release-dev/ocp-v4.0-art-dev@sha256:...",
"quay.io/openshift-release-dev/ocp-v4.0-art-dev@sha256:...",
...
]

These image references should all be by digest, tags aren't allowed.
items:
properties:
name:
description: |-
name is an OCI Image referenced by digest.

The format of the image ref is:
host[:port][/namespace]/name@sha256:<digest>
maxLength: 447
minLength: 1
type: string
x-kubernetes-validations:
- message: the OCI Image reference must end with a valid '@sha256:<digest>'
suffix, where '<digest>' is 64 characters long
rule: self.split('@').size() == 2 && self.split('@')[1].matches('^sha256:[a-f0-9]{64}$')
- message: the OCI Image name should follow the host[:port][/namespace]/name
format, resembling a valid URL without the scheme
rule: self.split('@')[0].matches('^([a-zA-Z0-9-]+\\.)+[a-zA-Z0-9-]+(:[0-9]{2,5})?/([a-zA-Z0-9-_]{0,61}/)?[a-zA-Z0-9-_.]*?$')
required:
- name
type: object
maxItems: 500
minItems: 1
type: array
x-kubernetes-list-map-keys:
- name
x-kubernetes-list-type: map
required:
- pinnedImages
type: object
status:
description: status describes the last observed state of this pinned image
set.
properties:
conditions:
description: conditions represent the observations of a pinned image
set's current state.
items:
description: Condition contains details for one aspect of the current
state of this API Resource.
properties:
lastTransitionTime:
description: |-
lastTransitionTime is the last time the condition transitioned from one status to another.
This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable.
format: date-time
type: string
message:
description: |-
message is a human readable message indicating details about the transition.
This may be an empty string.
maxLength: 32768
type: string
observedGeneration:
description: |-
observedGeneration represents the .metadata.generation that the condition was set based upon.
For instance, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration is 9, the condition is out of date
with respect to the current state of the instance.
format: int64
minimum: 0
type: integer
reason:
description: |-
reason contains a programmatic identifier indicating the reason for the condition's last transition.
Producers of specific condition types may define expected values and meanings for this field,
and whether the values are considered a guaranteed API.
The value should be a CamelCase string.
This field may not be empty.
maxLength: 1024
minLength: 1
pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$
type: string
status:
description: status of the condition, one of True, False, Unknown.
enum:
- "True"
- "False"
- Unknown
type: string
type:
description: type of condition in CamelCase or in foo.example.com/CamelCase.
maxLength: 316
pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$
type: string
required:
- lastTransitionTime
- message
- reason
- status
- type
type: object
type: array
x-kubernetes-list-map-keys:
- type
x-kubernetes-list-type: map
type: object
required:
- spec
type: object
served: true
storage: true
subresources:
status: {}
Loading