-
Notifications
You must be signed in to change notification settings - Fork 484
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
on-cluster builds enhancement #1515
base: master
Are you sure you want to change the base?
on-cluster builds enhancement #1515
Conversation
Skipping CI for Draft Pull Request. |
56bfc1d
to
01cb9d4
Compare
|
||
#### Device Drivers For Specialty Hardware | ||
|
||
As an OpenShift cluster administrator, I want to install a device driver |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could mention day 0 possibity here
content to the OS on each of their cluster nodes. | ||
|
||
2. The cluster administrator edits the MachineConfigPool, adding a | ||
Dockerfile to the appropriate field, image registry details such as |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clarify the purpose of the image registry and where it could be.
3. The BuildController detects that these details are provided and | ||
begins a build. | ||
|
||
4. The built image is pushed to the cluster administrators’ image |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see it covered here.
registry of choice. | ||
|
||
5. NodeController updates the desiredImage annotation on each node | ||
within its pool according to its own rules. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to clarify what we mean by rules here.
or annotation to signal to the MCO that it should consider these | ||
ConfigMaps as it prepares the rendered MachineConfig. | ||
|
||
- What annotations will bootc support for ConfigMaps? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd like to try to track this stuff is best tracked in upstream discussions in bootc. It's really up to us! IOW we need to drive the design there so that it meets these use cases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fine with me! I added some strawmans to this enhancement to get us started but I'm happy to add them someplace else.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I put a slightly reduced set of my ConfigMap strawmans from this enhancement in containers/bootc#22.
As an OpenShift cluster administrator, I want to prepare my customized | ||
OS content and configuration changes ahead of time so that I can reduce | ||
or eliminate issues with deploying them. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#### Managed OpenShift Service Provider | |
As a Managed OpenShift SRE, I want to install cloud specific tooling to ensure SRE can triage issues with all nodes involved in the lifecycle of a Managed OpenShift cluster, including bootstrap, without requiring any reboot/pivot. | |
#### Managed OpenShift Service Consumer | |
As a Managed OpenShift service consumer, I want to know any changes to node configuration can be tested and validated against a known set of criteria so I know my nodes will not break support agreements with Managed OpenShift Service Providers. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for these additional stories! I'll get them added.
upgraded to a new OpenShift release, any custom OS content will be | ||
upgraded as well (subject to update availability). | ||
|
||
- A cluster administrator will have the option to build their custom OS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- A cluster administrator will have the option to build their custom OS | |
- A cluster administrator will have the option to build and test their custom OS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for catching that!
orchestrate more than one build for each target architecture as well as | ||
manifestlisting the final image. This consideration also extends to the | ||
image registry the final image will be pushed to. To my understanding, | ||
the default on-cluster image registry does not support manifestlisted |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the internal image registry does support pushing and pulling manifestlists now . Imagestream imports was not working with manifestlists and that has been introduced since 4.13
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's great to know! I'll adjust the enhancement accordingly. I'll also do some separate experimentation around this once we begin looking at multiarch support.
|
||
BuildController should be as modular as possible with how it performs | ||
the build to allow it to efficiently operate in as many different | ||
environments as possible. For example, if the OpenShift Image Builds |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unfortunately the builds v1 api does not support building manifestlisted images so this will have to be homegrown as is suggested.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I figured that would be the case. I've done some experimentation around doing something like that in the past, so it was definitely a useful exercise. I'll add some context around that being a requirement.
Inactive enhancement proposals go stale after 28d of inactivity. See https://github.com/openshift/enhancements#life-cycle for details. Mark the proposal as fresh by commenting If this proposal is safe to close now please do so with /lifecycle stale |
Stale enhancement proposals rot after 7d of inactivity. See https://github.com/openshift/enhancements#life-cycle for details. Mark the proposal as fresh by commenting If this proposal is safe to close now please do so with /lifecycle rotten |
Rotten enhancement proposals close after 7d of inactivity. See https://github.com/openshift/enhancements#life-cycle for details. Reopen the proposal by commenting /close |
@openshift-bot: Closed this PR. 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 kubernetes/test-infra repository. |
(automated message) This pull request is closed with lifecycle/rotten. The associated Jira ticket, MCO-834, has status "Code Review". Should the PR be reopened, updated, and merged? If not, removing the lifecycle/rotten label will tell this bot to ignore it in the future. |
/reopen |
@cheesesashimi: Reopened this PR. 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 kubernetes/test-infra repository. |
@jlebon: Reopened this PR. 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 kubernetes-sigs/prow repository. |
Inactive enhancement proposals go stale after 28d of inactivity. See https://github.com/openshift/enhancements#life-cycle for details. Mark the proposal as fresh by commenting If this proposal is safe to close now please do so with /lifecycle stale |
Stale enhancement proposals rot after 7d of inactivity. See https://github.com/openshift/enhancements#life-cycle for details. Mark the proposal as fresh by commenting If this proposal is safe to close now please do so with /lifecycle rotten |
Rotten enhancement proposals close after 7d of inactivity. See https://github.com/openshift/enhancements#life-cycle for details. Reopen the proposal by commenting /close |
@openshift-bot: Closed this PR. 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 kubernetes-sigs/prow repository. |
/reopen |
@JoelSpeed: Reopened this PR. 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 kubernetes-sigs/prow repository. |
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 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 |
/remove-lifecycle rotten Will look to update this this week |
These updates were authored by @cheesesashimi and @umohnani8, with my part solely being editorial here. Co-authored-by: Zack Zlotnik <[email protected]> Co-authored-by: Urvashi Mohnani <[email protected]>
b9cce87
to
ebb5968
Compare
Enhancement should now be updated for the latest state of APIs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left a few questions/updates. I'm guessing we will want to update this again based on how the removal of injecting MachineConfig
contents into the image changes things.
ReleaseOSImageChange--> |OpenShift upgrade changed \n the OS base image| IncomingConfigChange | ||
MachineConfigChange--> |How MachineConfigs are updated now| IncomingConfigChange | ||
CustomOSContentChange--> |The Containerfile has changed| IncomingConfigChange | ||
CustomBaseOSImageChange--> |The cluster admin changed the base image pullspec| IncomingConfigChange |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think v1 MachineOSConfig
allows a change to the base OS image, so this branch can probably go away? Or are we saying a user could update OSImageURL
via MC, hence overriding the base image for the build?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think in the flowchart this no longer makes sense, removed!
@@ -274,7 +268,6 @@ MachineOSConfigs can also be augmented with the following annotations: | |||
| `machineconfiguration.openshift.io/rebuildImage` | When present, this will clear any failed image builds (and any supporting transitory objects created) and retry the build. | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this annotation has changed into machineconfiguration.openshift.io/rebuild
@@ -274,7 +268,6 @@ MachineOSConfigs can also be augmented with the following annotations: | |||
| `machineconfiguration.openshift.io/rebuildImage` | When present, this will clear any failed image builds (and any supporting transitory objects created) and retry the build. | | |||
| `machineconfiguration.openshift.io/noRollout` | When present, this will perform the image build, but will not roll out the built image to the nodes within the targeted MachineConfigPool. | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this annotation is used anymore, probably because it mirrored pool pausing behavior
@@ -476,23 +446,23 @@ flowchart TB | |||
BootcConfigChange("bootc ConfigMap Change") | |||
MachineConfigChange("MachineConfig Change") | |||
CustomOSContentChange("Custom OS Content Change") | |||
CustomBaseOSImageChange("Admin-Defined Base OS \n Image Change") | |||
ReleaseOSImageChange("OpenShift Release \n Base OS Image Change") | |||
CustomBaseOSImageChange("Admin-Defined Base OS Image Change") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same question as previous flowchart
Updated content based on latest state: - Updated MachineOSConfig and MachineOSBuild references and charts - Removed reference to MachineOSImage - Removed reference to MCP extensions - Updated pod reference to Jobs - Added some snippets to graduation plans
ebb5968
to
0ba7877
Compare
@cheesesashimi: all tests passed! 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. |
No description provided.