-
Notifications
You must be signed in to change notification settings - Fork 508
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-821: Introduce MachineConfigNode types #1596
Conversation
Hello @cdoern! Some important instructions when contributing to openshift/api: |
000e721
to
d5de927
Compare
machineconfiguration/v1/types.go
Outdated
|
||
// MachineState describes the health of the Machines on the system | ||
// | ||
// Compatibility level 1: Stable within a major release for a minimum of 12 months or 3 minor releases (whichever is longer). |
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.
born into compat-level 1 is very confident. Are we already that confident that we are getting these types right, and won't need tough follow-up changes?
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.
not sure what the protocol is here. I was under the impression that anything user facing should be level 1?
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.
If we are going to use a feature gate for this, which I think we should try, then you want to make this a v1alpha1 API first, at compatibility level 4
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 am planning on featuregating, do you think I need to introduce that in this PR?
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.
We can do the feature gate as a separate PR, that's a pretty easy thing to add so can probably get that in and merged before the API
/cc |
/lgtm I would like some acknowledgement from others on the EP that they are happy for the API to merge even though the EP still has some details to iron out, have posted a comment over on the EP openshift/enhancements#1490 (comment) |
/test e2e-upgrade |
728764d
to
ea94212
Compare
/approve cancel I think the description of the spec and status desired versions are the wrong way around, can we check |
Signed-off-by: Charlie Doern <cdoern@redhat.com>
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cdoern, JoelSpeed, yuqi-zhang 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 |
/test all |
/hold cancel We have enough consensus to get this in, and iterate with what's left of 4.15 |
/test integration |
/test e2e-aws-serial-techpreview |
@cdoern: 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/test-infra repository. I understand the commands that are listed here. |
For Openshift 4.15, the MCO is working on a new state reporting mechanism: The MachineConfigStateController. The main type behind this architecture is the MachineConfigNode. A MachineConfigNode tracks an upgrade progression that relates behaviors between a node, its machineconfigpool, and the MCD processes within the MCO.
Each of these will have its own CR in the MCO which necessitates a CRD that I have also included in this PR.
where doing
oc describe machineconfignode/<name>
will of course display more in depth information of the typemetav1.Condition
. These conditions are composed of a state, phase, reason, and time. The state refers to what part of the upgrade we are in (UpdatePreparing, Completing etc), Phase is a in depth description of what part of a State we are in, and the Reason is a quick one or two word statement describing what "Stopping Point" we are at: ReconcilingConfigs, StoppingCfgDrift. The following accepted Condition types are:The standard
oc get machineconfignode/...
will display onlyUpdated
UpdatePreparing
UpdateInProgress
UpdatePostAction
UpdateCompleting
andResuming
. The rest of these granular states will be shown via-o wide
oroc describe
An example of
oc get machineconfignodes
would look like:The other information accessible via
oc describe
will be:this is an example of the above
oc describe
command that is not fully operational yet.As well as this information located at the bottom of
oc describe
This feature is linked to https://issues.redhat.com/browse/MCO-452 and more detail can be found there.
These types are basically an attempt to aggregate and display the state of a cluster's machines in a user friendly or at least digestible way. Each Condition is also associated closely with a StateProgress. All of these StateProgresses are phases that I have broken down key MCO operations into.
The structure and types of this PR are being used in: openshift/machine-config-operator#3970 which is working quite well.