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-993: MachineOSBuild API #1773

Merged
merged 1 commit into from
Apr 5, 2024
Merged

Conversation

cdoern
Copy link
Contributor

@cdoern cdoern commented Feb 20, 2024

machineOSBuild and Image are meant to represent the two components of an on cluster build, making this commit the user-facing API for on cluster builds.

MachineOSBuilds comprise of the actual processes of kicking off an monitoring a build. A build also contains user options like the push/pull spec for registries as well as the custom content in a containerfile. The state of a build will be tracked in here as well, and users will be able to view this in printer columns. A build is either: "Ready", "BuildPrepared", "Building", "BuildFailed", "BuildInterrupted", or "BuildRestarted"

As of 4.16, the MCO will migrate the currently implemented OCB functionality to this API. We are not looking to add new functionality in 4.16. However, in 4.17 specific elements of this API will most likely be pre-populated by work in the MCO. Where we can, we are looking to provide the user with all of the base image pull secrets, and final image push and pull secrets. For now, these must be user provided.

Copy link
Contributor

openshift-ci bot commented Feb 20, 2024

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 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 Feb 20, 2024
Copy link
Contributor

openshift-ci bot commented Feb 20, 2024

Hello @cdoern! 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 openshift-ci bot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Feb 20, 2024
@cdoern cdoern changed the title machineOSBuild and machineOSImage API MCO-993: machineOSBuild and machineOSImage API Feb 20, 2024
@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Feb 20, 2024
@openshift-ci-robot
Copy link

openshift-ci-robot commented Feb 20, 2024

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

In response to this:

machineOSBuild and Image are meant to represent the two components of an on cluster build, making this commit the user-facing API for on cluster builds.

MOSBuilds comprise of the actual processes of kicking off an monitoring a build. A build also contains user options like the push/pull spec for registries as well as the custom content in a containerfile. The state of a build will be tracked in here as well, and users will be able to view this in printer columns. A build is either: "Built", "BuildPrepared", "Building", "BuildFailed", "BuildInterrupted", or "BuildRestarted"

a MOSImage is the result of a build. the Image objects are intended to be trackable and easily related to an MCP. The goal here is to remove much of the split state reporting for on cluster builds and consolidate where the user sets config options. These images have status conditions as well related to their Age and their Rollout to the nodes on a pool. The valid conditions are "RollingOut", "RolledOut", "RolloutFailed". There is also an "Age" printer column detailing the age of the image.

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.

@cdoern
Copy link
Contributor Author

cdoern commented Feb 20, 2024

/jira refresh

@openshift-ci-robot
Copy link

openshift-ci-robot commented Feb 20, 2024

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

In response to this:

/jira refresh

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.

@cdoern cdoern force-pushed the build branch 2 times, most recently from a5708c3 to beb81e8 Compare February 28, 2024 14:47
@cdoern cdoern changed the title MCO-993: machineOSBuild and machineOSImage API MCO-993: MachineOSBuild API Feb 28, 2024
@openshift-ci-robot
Copy link

openshift-ci-robot commented Feb 28, 2024

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

In response to this:

machineOSBuild and Image are meant to represent the two components of an on cluster build, making this commit the user-facing API for on cluster builds.

MOSBuilds comprise of the actual processes of kicking off an monitoring a build. A build also contains user options like the push/pull spec for registries as well as the custom content in a containerfile. The state of a build will be tracked in here as well, and users will be able to view this in printer columns. A build is either: "Built", "BuildPrepared", "Building", "BuildFailed", "BuildInterrupted", or "BuildRestarted"

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.

@cdoern cdoern marked this pull request as ready for review February 28, 2024 14:49
@openshift-ci 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. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. labels Feb 28, 2024
@cdoern cdoern force-pushed the build branch 4 times, most recently from 102ced6 to 86124de Compare February 29, 2024 16:25
Copy link
Member

@cheesesashimi cheesesashimi left a comment

Choose a reason for hiding this comment

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

Overall, this is a great first pass! I do have a lot of questions and thoughts about things. I'm happy to discuss them further if anything is unclear.

machineconfiguration/v1alpha1/types_machineosbuild.go Outdated Show resolved Hide resolved
machineconfiguration/v1alpha1/types_machineosbuild.go Outdated Show resolved Hide resolved
machineconfiguration/v1alpha1/types_machineosbuild.go Outdated Show resolved Hide resolved
machineconfiguration/v1alpha1/types_machineosbuild.go Outdated Show resolved Hide resolved
machineconfiguration/v1alpha1/types_machineosbuild.go Outdated Show resolved Hide resolved
machineconfiguration/v1alpha1/types_machineosbuild.go Outdated Show resolved Hide resolved
machineconfiguration/v1alpha1/types_machineosbuild.go Outdated Show resolved Hide resolved
machineconfiguration/v1alpha1/types_machineosbuild.go Outdated Show resolved Hide resolved
machineconfiguration/v1alpha1/types_machineosbuild.go Outdated Show resolved Hide resolved
machineconfiguration/v1alpha1/types_machineosbuild.go Outdated Show resolved Hide resolved
machineconfiguration/v1alpha1/types_machineosbuild.go Outdated Show resolved Hide resolved
machineconfiguration/v1alpha1/types_machineosbuild.go Outdated Show resolved Hide resolved
machineconfiguration/v1alpha1/types_machineosbuild.go Outdated Show resolved Hide resolved
machineconfiguration/v1alpha1/types_machineosbuild.go Outdated Show resolved Hide resolved
machineconfiguration/v1alpha1/types_machineosbuild.go Outdated Show resolved Hide resolved
machineconfiguration/v1alpha1/types_machineosbuild.go Outdated Show resolved Hide resolved
machineconfiguration/v1alpha1/types_machineosbuild.go Outdated Show resolved Hide resolved
machineconfiguration/v1alpha1/types_machineosbuild.go Outdated Show resolved Hide resolved
@openshift-ci openshift-ci bot removed the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Mar 4, 2024
@deads2k
Copy link
Contributor

deads2k commented Apr 5, 2024

/approve

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 5, 2024
Copy link
Member

@cheesesashimi cheesesashimi left a comment

Choose a reason for hiding this comment

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

Almost there! Just a couple very minor things left.

machineconfiguration/v1alpha1/types_machineosconfig.go Outdated Show resolved Hide resolved
machineconfiguration/v1alpha1/types_machineosconfig.go Outdated Show resolved Hide resolved
// describes the x86_64 architecture
X86_64 ContainerFileArch = "x86_64"
// describes a containerfile that can be applied to any arch
NoArch ContainerFileArch = "noArch"
Copy link
Member

Choose a reason for hiding this comment

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

suggestion: For consistency, the value for NoArch should probably be noarch since that's the convention that dnf / yum use for packages which work on all architectures.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@cheesesashimi
Copy link
Member

/lgtm
/approve

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Apr 5, 2024
Copy link
Contributor

openshift-ci bot commented Apr 5, 2024

@cdoern: The following test 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-upgrade-minor 4bfcc9c link true /test e2e-upgrade-minor

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

machineOSBuild and Image are meant to represent the two components of an on cluster build, making this commit the
user-facing API for on cluster builds. MOSBuilds comprise of the actual processes of kicking off an monitoring a build.

A MachineOSConfig contains user options like the push/pull spec for registries as well as the custom content in a dockerfile.  a MOSImage
is the result of a build. the Image objects are intended to be trackable and easily related to an MCP. The goal here is to remove much of the
split state reporting for on cluster builds and consolidate where the user sets config options.

Signed-off-by: Charlie Doern <cdoern@redhat.com>
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Apr 5, 2024
@cheesesashimi
Copy link
Member

/lgtm
/approve

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Apr 5, 2024
Copy link
Contributor

openshift-ci bot commented Apr 5, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cdoern, cheesesashimi, deads2k

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-bot openshift-merge-bot bot merged commit abd990c into openshift:master Apr 5, 2024
9 checks passed
@openshift-bot
Copy link

[ART PR BUILD NOTIFIER]

This PR has been included in build ose-cluster-config-api-container-v4.16.0-202404052115.p0.gabd990c.assembly.stream.el9 for distgit ose-cluster-config-api.
All builds following this will include this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants