MCO-1443: Promote onclusterbuild to GA #2090
Conversation
|
Skipping CI for Draft Pull Request. |
|
Hello @yuqi-zhang! Some important instructions when contributing to openshift/api: |
openshift-ci
bot
added
do-not-merge/work-in-progress
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: yuqi-zhang The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/test all |
yuqi-zhang
force-pushed
the
promote-onclusterbuild
branch
from
8465feb
to
2ca4842
Compare
|
/test all |
First step to GA'ing the currently v1alpha1 APIs. Don't add to payload manifests yet, and the featuregate is retained.
yuqi-zhang
force-pushed
the
promote-onclusterbuild
branch
from
2ca4842
to
4c9f154
Compare
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
yuqi-zhang
changed the title
|
@yuqi-zhang: This pull request references MCO-1443 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.18.0" version, but no target version was set. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
openshift-ci-robot
added
the
jira/valid-reference
| @@ -0,0 +1,64 @@ | |||
| apiVersion: apiextensions.k8s.io/v1 # Hack because controller-gen complains if we don't have this | |||
| name: "[TechPreview] MachineOSBuild" | |||
There was a problem hiding this comment.
No longer tech preview? I know it kind of is, but, we aren't likely to remember to update this when we update the gate, so this will become disjoint
| @@ -0,0 +1,134 @@ | |||
| apiVersion: apiextensions.k8s.io/v1 # Hack because controller-gen complains if we don't have this | |||
| name: "[TechPreview] MachineOSConfig" | |||
| // version tracks the newest MachineOSBuild for each MachineOSConfig | ||
| // +kubebuilder:validation:Minimum=1 | ||
| // +kubebuilder:validation:Required | ||
| Version int64 `json:"version"` |
There was a problem hiding this comment.
I'm not following how this interoperates, in particular, with ConfigGeneration? Can you remind me?
There was a problem hiding this comment.
I think the original intent was that they track a corresponding MachineOSConfig generation to do rebuilds, but in techpreview the implementation hasn't leveraged either field (hard set to 1). Let me check that and get back to you.
| // host[:port][/namespace]/name:<tag> or svc_name.namespace.svc[:port]/repository/name:<tag> | ||
| // +kubebuilder:validation:MinLength=1 | ||
| // +kubebuilder:validation:MaxLength=447 | ||
| // +kubebuilder:validation:XValidation:rule=`((self.split(':').size() == 2 && self.split(':')[1].matches('^([a-zA-Z0-9-./:])+$')) || self.matches('^[^.]+\\.[^.]+\\.svc:\\d+\\/[^\\/]+\\/[^\\/]+:[^\\/]+$'))`,message="the OCI Image reference must end with a valid :<tag>, where '<digest>' is 64 characters long and '<tag>' is any valid string Or it must be a valid .svc followed by a port, repository, image name, and tag." |
There was a problem hiding this comment.
The message here references digest, but it's not in the example?
| // +listMapKey=type | ||
| // +optional | ||
| Conditions []metav1.Condition `json:"conditions,omitempty" patchStrategy:"merge" patchMergeKey:"type"` | ||
| // ImageBuilderType describes the image builder set in the MachineOSConfig |
There was a problem hiding this comment.
Godoc for this field is wrong, it's talking about a different field name?
| // name is the name of the secret used to push or pull this MachineOSConfig object. | ||
| // this secret must be in the openshift-machine-config-operator namespace. | ||
| // +kubebuilder:validation:Required | ||
| Name string `json:"name"` |
There was a problem hiding this comment.
CEL format validation here too
| // +kubebuilder:default:=noarch | ||
| // +optional | ||
| ContainerfileArch ContainerfileArch `json:"containerfileArch"` | ||
| // content is the custom content to be built |
There was a problem hiding this comment.
What is this? Can we expand this documentation?
|
|
||
| type MachineOSImageBuilder struct { | ||
| // imageBuilderType specifies the backend to be used to build the image. | ||
| // +kubebuilder:default:=PodImageBuilder |
There was a problem hiding this comment.
Are we sure we want this defaulted? What if we need to change this in the future as we decide there's a better, default image build method that doesn't rely on today's pod based image builder?
There was a problem hiding this comment.
Is it difficult to change the default here in the API if we wanted to change it in the future?
| // +listType=map | ||
| // +listMapKey=containerfileArch | ||
| // +kubebuilder:validation:MinItems=0 | ||
| // +kubebuilder:validation:MaxItems=7 |
There was a problem hiding this comment.
This should be explained in the godoc
| // +kubebuilder:validation:XValidation:rule=`((self.split(':').size() == 2 && 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-_.]*?$')) || self.matches('^[^.]+\\.[^.]+\\.svc:\\d+\\/[^\\/]+\\/[^\\/]+:[^\\/]+$'))`,message="the OCI Image name should follow the host[:port][/namespace]/name format, resembling a valid URL without the scheme. Or it must be a valid .svc followed by a port, repository, image name, and tag." | ||
| // +kubebuilder:validation:Required | ||
| RenderedImagePushspec string `json:"renderedImagePushspec"` | ||
| // releaseVersion is associated with the base OS Image. This is the version of Openshift that the Base Image is associated with. |
There was a problem hiding this comment.
This line almost says the same thing twice, perhaps this should consolidate into one?
yuqi-zhang
force-pushed
the
promote-onclusterbuild
branch
from
dfedb21
to
a7428f4
Compare
|
@yuqi-zhang: The following tests failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
Opened for testing. Based on guidance this is currently the first step: create new v1 API, gate remains off, v1 API is excluded from the image manifests
Also adds in: #2089