[wip] add PinnedImageSet types and crd by hexfusion · Pull Request #1713 · openshift/api

hexfusion · 2024-01-02T23:00:32Z

This PR adds PinnedImageSet types to the machineconfiguration.openshift.io group and a CRD as a continuation of #1609. More will be needed but a little time is needed to finalize final status requirements and this gets us in the door.

ref:

MCO-838
updated enhancement enhancements/machine-config: add updates to PinnedImageSet enhancements#1599

openshift-ci · 2024-01-02T23:00:37Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

openshift-ci · 2024-01-02T23:00:38Z

Hello @hexfusion! Some important instructions when contributing to openshift/api:
API design plays an important part in the user experience of OpenShift and as such API PRs are subject to a high level of scrutiny to ensure they follow our best practices. If you haven't already done so, please review the OpenShift API Conventions and ensure that your proposed changes are compliant. Following these conventions will help expedite the api review process for your PR.

openshift-ci-robot · 2024-01-03T15:26:47Z

@hexfusion: This pull request references MCO-838 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 epic to target the "4.16.0" version, but no target version was set.

Details

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 · 2024-01-03T15:29:09Z

@hexfusion: This pull request references MCO-838 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 epic to target the "4.16.0" version, but no target version was set.

Details

In response to this:

This PR adds PinnedImageSet types to the machineconfiguration.openshift.io group and a CRD as a continuation of #1609.

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.

hexfusion · 2024-01-03T15:51:40Z

/test all

openshift-ci-robot · 2024-01-04T17:00:44Z

@hexfusion: This pull request references MCO-838 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 epic to target the "4.16.0" version, but no target version was set.

Details

In response to this:

This PR adds PinnedImageSet types to the machineconfiguration.openshift.io group and a CRD as a continuation of #1609.

ref: MCO-838

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 · 2024-01-04T17:00:52Z

@hexfusion: This pull request references MCO-838 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 epic to target the "4.16.0" version, but no target version was set.

Details

In response to this:

This PR adds PinnedImageSet types to the machineconfiguration.openshift.io group and a CRD as a continuation of #1609.

ref: MCO-838

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 · 2024-01-10T23:30:42Z

@hexfusion: This pull request references MCO-838 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 epic to target the "4.15.0" version, but no target version was set.

Details

In response to this:

This PR adds PinnedImageSet types to the machineconfiguration.openshift.io group and a CRD as a continuation of #1609.

ref: MCO-838

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 · 2024-01-11T00:05:44Z

@hexfusion: This pull request references MCO-838 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 epic to target the "4.15.0" version, but no target version was set.

Details

In response to this:

This PR adds PinnedImageSet types to the machineconfiguration.openshift.io group and a CRD as a continuation of #1609.

ref: MCO-838

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.

hexfusion · 2024-01-11T00:06:15Z

/test all

hexfusion · 2024-03-14T13:37:24Z

Wondering where are we going to track that now? Will it be incorporated into MachineConfigNode now?

Pre-fetching sounds like a node level thing right? So I would expect MCN to host the status for each node's pre-fetch

The implementation will be very tightly coupled with MCP and MCD. it will not add additional node annotations instead use those already used by MCD.

So the goal will be to pivot to MCN when it becomes v1 but not before.

JoelSpeed · 2024-03-15T10:33:27Z

+	// pinnedImageSets specifies a sequence of PinnedImageSetRef objects for the
+	// pool. Nodes within this pool will preload and pin images defined in the
+	// PinnedImageSet. Before pulling images the controller will ensure the
+	// total uncompressed size of all the images does not exceed available


What happens if we do exceed the available disk space?

controller does a precheck calculating the storage required for all images + buffer ~20GB - allocatable, before we pull. If this fails the pool goes degraded. The node is checked for KubeletHasNoDiskPressure if that at anytime is true controller workers will bail and retry if that can't be reconciled eventually it will be Unreconcilable. Also the node will become NotReady as a result of disk pressure (pretty sure).

Kubelet would force a GC on images(need to verify) and potentially evict pods. On the control-plane worst case it could take the cluster down. So there is a fair amount of assumption that the kubelet reports allocatable storage correctly.

For this reason, we will not pin the images until all images are pulled to allow the Kubelet the ability to GC>

openshift-ci-robot · 2024-03-15T17:20:28Z

@hexfusion: This pull request references MCO-838 which is a valid jira issue.

Details

In response to this:

This PR adds PinnedImageSet types to the machineconfiguration.openshift.io group and a CRD as a continuation of #1609.

ref:

MCO-838

updated enhancement enhancements/machine-config: add updates to PinnedImageSet enhancements#1599

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.

Signed-off-by: Sam Batschelet <sbatsche@redhat.com>

hexfusion · 2024-03-15T17:32:43Z

make verify is failing in the new Custom/TechPreview crd for v1 unrelated to changes directly in this PR

error running generator schemacheck on group machineconfiguration.openshift.io: 
        could not run schemacheck generator for group/version machineconfiguration.openshift.io/v1: 
                error in 0000_80_machineconfigpool-CustomNoUpgrade.crd.yaml: ListsMustHaveSSATags: crd/machineconfigpools.machineconfiguration.openshift.io version/v1 field/^.spec.machineConfigSelector.matchExpressions must set x-kubernetes-list-type
                error in 0000_80_machineconfigpool-CustomNoUpgrade.crd.yaml: ListsMustHaveSSATags: crd/machineconfigpools.machineconfiguration.openshift.io version/v1 field/^.spec.machineConfigSelector.matchExpressions[*].values must set x-kubernetes-list-type
`

hexfusion · 2024-03-15T18:34:42Z

0000_80_machineconfigpool-Default.crd.yaml

Do I want this? It seems recent updates now generate this file, perhaps it should replace the other stable CRD?

Signed-off-by: Sam Batschelet <sbatsche@redhat.com>

hexfusion · 2024-03-15T21:24:29Z

In some cases you may have changed or renamed a file that already contained api violations, but you are not introducing a new violation. In such cases it is appropriate to /override the failing CI job.

:)

Signed-off-by: Sam Batschelet <sbatsche@redhat.com>

JoelSpeed

Nit on the tests. Would be good to test the name validation in the references with both valid and non valid cases, but otherwise the API shape is good here

JoelSpeed · 2024-03-18T13:31:57Z

+        pinnedImageSets:
+          - name: test-pinnedimageset


This isn't a required field is it? Generally the minimal version of this test shows only required fields

ok gotcha so I can add an additional test to show this. Will update

yuqi-zhang

Overall makes sense to me from the MCO perspective. Do you foresee the need to add any new status types?

yuqi-zhang · 2024-03-18T22:33:01Z

+	// +kubebuilder:validation:MinLength=1
+	// +kubebuilder:validation:MaxLength=447
+	// +kubebuilder:validation:XValidation:rule=`self.matches('^([a-zA-Z0-9-]+\\.)+[a-zA-Z0-9-]+(:[0-9]{2,5})?/([a-zA-Z0-9-_]{0,61}/)?[a-zA-Z0-9-_.]*@sha256:[a-f0-9]{64}$')`,message="The OCI Image reference must be in the format host[:port][/namespace]/name@sha256:<digest> with a valid SHA256 digest"
+	Name string `json:"name"`


I'm wondering if there were alternatives to just "name" here, as this is more of a... url? digest? I guess I don't have a good alternative

openshift-ci · 2024-03-19T13:00:02Z

@hexfusion: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/e2e-aws-serial-techpreview	`5541689`	link	true	`/test e2e-aws-serial-techpreview`
ci/prow/verify-crd-schema	`b97feaf`	link	true	`/test verify-crd-schema`
ci/prow/e2e-upgrade-minor	`b97feaf`	link	true	`/test e2e-upgrade-minor`

Full PR test history. Your PR dashboard.

Details

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/test-infra repository. I understand the commands that are listed here.

openshift-merge-robot · 2024-03-19T13:00:06Z

PR needs rebase.

Details

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/test-infra repository.

hexfusion · 2024-03-21T13:48:13Z

THis PR evolved a lot over time as the design drifted away from the enhancement while this is good historical context closing and moving to #1822

openshift-ci-robot · 2024-03-21T13:48:24Z

@hexfusion: No Jira issue is referenced in the title of this pull request.
To reference a jira issue, add 'XYZ-NNN:' to the title of this pull request and request another refresh with /jira refresh.

Details

In response to this:

This PR adds PinnedImageSet types to the machineconfiguration.openshift.io group and a CRD as a continuation of #1609.

ref:

MCO-838

updated enhancement enhancements/machine-config: add updates to PinnedImageSet enhancements#1599

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 Bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 2, 2024

openshift-ci Bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jan 2, 2024

hexfusion force-pushed the add_pinned_image_set branch from bcfe707 to f2dc1f4 Compare January 3, 2024 15:23

hexfusion changed the title ~~add PinnedImageSet types and crd~~ MCO-838: add PinnedImageSet types and crd Jan 3, 2024

openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Jan 3, 2024

hexfusion mentioned this pull request Jan 3, 2024

Add PinnedImageSet #1609

Closed

hexfusion commented Jan 5, 2024

View reviewed changes

Comment thread machineconfiguration/v1/types.go Outdated

hexfusion changed the base branch from master to release-4.15 January 10, 2024 23:30

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 10, 2024

openshift-ci Bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jan 10, 2024

hexfusion force-pushed the add_pinned_image_set branch from 6cb5940 to 1d2ec1c Compare January 10, 2024 23:58

openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 10, 2024

hexfusion closed this Jan 11, 2024

hexfusion force-pushed the add_pinned_image_set branch from 1d2ec1c to 44756aa Compare January 11, 2024 00:02

openshift-ci Bot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jan 11, 2024

hexfusion reopened this Jan 11, 2024

openshift-ci Bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jan 11, 2024

sinnykumari reviewed Mar 14, 2024

View reviewed changes

Comment thread machineconfiguration/v1/types.go Outdated

openshift-ci Bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 14, 2024

hexfusion force-pushed the add_pinned_image_set branch 3 times, most recently from c286509 to 9220483 Compare March 15, 2024 03:25

openshift-ci Bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Mar 15, 2024

hexfusion force-pushed the add_pinned_image_set branch 2 times, most recently from 44309e5 to 363d8b8 Compare March 15, 2024 03:31

openshift-ci Bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Mar 15, 2024

JoelSpeed reviewed Mar 15, 2024

View reviewed changes

hexfusion added 3 commits March 15, 2024 13:21

machineconfiguration/v1alpha1: add PinnedImageSet

7ed8dfb

Signed-off-by: Sam Batschelet <sbatsche@redhat.com>

feature gate v1 types

079ff78

Signed-off-by: Sam Batschelet <sbatsche@redhat.com>

updates from review and regen

de8b7bf

Signed-off-by: Sam Batschelet <sbatsche@redhat.com>

update docs and regen crds

ca4d81e

Signed-off-by: Sam Batschelet <sbatsche@redhat.com>

align RFC-1123 regex with existing

b97feaf

Signed-off-by: Sam Batschelet <sbatsche@redhat.com>

JoelSpeed reviewed Mar 18, 2024

View reviewed changes

yuqi-zhang reviewed Mar 18, 2024

View reviewed changes

hexfusion mentioned this pull request Mar 21, 2024

MCO-838: machineconfiguration/v1alpha1: add PinnedImageSet #1822

Merged

Conversation

hexfusion commented Jan 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci Bot commented Jan 2, 2024

Uh oh!

openshift-ci Bot commented Jan 2, 2024

Uh oh!

openshift-ci-robot commented Jan 3, 2024 • edited by openshift-ci Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci-robot commented Jan 3, 2024 • edited by openshift-ci Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hexfusion commented Jan 3, 2024

Uh oh!

openshift-ci-robot commented Jan 4, 2024 • edited by openshift-ci Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci-robot commented Jan 4, 2024 • edited by openshift-ci Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

openshift-ci-robot commented Jan 10, 2024 • edited by openshift-ci Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci-robot commented Jan 11, 2024 • edited by openshift-ci Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hexfusion commented Jan 11, 2024

Uh oh!

Uh oh!

hexfusion commented Mar 14, 2024

Uh oh!

JoelSpeed Mar 15, 2024

Choose a reason for hiding this comment

Uh oh!

hexfusion Mar 15, 2024

Choose a reason for hiding this comment

Uh oh!

hexfusion Mar 15, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

openshift-ci-robot commented Mar 15, 2024 • edited by openshift-ci Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hexfusion commented Mar 15, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hexfusion commented Mar 15, 2024

Uh oh!

hexfusion commented Mar 15, 2024

Uh oh!

JoelSpeed left a comment

Choose a reason for hiding this comment

Uh oh!

JoelSpeed Mar 18, 2024

Choose a reason for hiding this comment

Uh oh!

hexfusion Mar 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yuqi-zhang left a comment

Choose a reason for hiding this comment

Uh oh!

yuqi-zhang Mar 18, 2024

Choose a reason for hiding this comment

Uh oh!

openshift-ci Bot commented Mar 19, 2024

Uh oh!

openshift-merge-robot commented Mar 19, 2024

Uh oh!

hexfusion commented Mar 21, 2024

Uh oh!

openshift-ci-robot commented Mar 21, 2024

Uh oh!

Reviewers

Assignees

hexfusion commented Jan 2, 2024 •

edited

Loading

openshift-ci-robot commented Jan 3, 2024 •

edited by openshift-ci Bot

Loading

openshift-ci-robot commented Jan 3, 2024 •

edited by openshift-ci Bot

Loading

openshift-ci-robot commented Jan 4, 2024 •

edited by openshift-ci Bot

Loading

openshift-ci-robot commented Jan 4, 2024 •

edited by openshift-ci Bot

Loading

openshift-ci-robot commented Jan 10, 2024 •

edited by openshift-ci Bot

Loading

openshift-ci-robot commented Jan 11, 2024 •

edited by openshift-ci Bot

Loading

openshift-ci-robot commented Mar 15, 2024 •

edited by openshift-ci Bot

Loading

hexfusion commented Mar 15, 2024 •

edited

Loading

hexfusion Mar 18, 2024 •

edited

Loading