Skip to content

*: Support CoreOS configs in MCs #1980

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.

Sign up for GitHub

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

Jump to bottom
Closed
LorbusChris wants to merge 2 commits into from
Closed

*: Support CoreOS configs in MCs #1980

LorbusChris wants to merge 2 commits into from

Conversation

@LorbusChris
Copy link
Contributor

@LorbusChris LorbusChris commented Aug 4, 2020 •
edited
Loading

- What I did
Add support for parsing CoreOS configs from RawExtension in MC

This is one possible way to go about the issue discussed in #283

- How to verify it
I'll add a test
- Description for the changelog
Support CoreOS configs in MCs

/hold

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 4, 2020
@LorbusChris
Copy link
Contributor Author

LorbusChris commented Aug 4, 2020

/retest

@kikisdeliveryservice
Copy link
Contributor

kikisdeliveryservice commented Aug 4, 2020

hi @LorbusChris is there any problem definition/background for this?

@LorbusChris LorbusChris force-pushed the fcct-in-mc branch 2 times, most recently from a400c3b to dac6bc7 Compare August 5, 2020 08:53
@LorbusChris
Copy link
Contributor Author

LorbusChris commented Aug 5, 2020

@kikisdeliveryservice this is one way to solve #283
It has come up a few times. Just threw this up here to open up the discussion.

@LorbusChris
Copy link
Contributor Author

LorbusChris commented Aug 5, 2020

/retest

@LorbusChris
Copy link
Contributor Author

LorbusChris commented Aug 6, 2020

/retest

@LorbusChris LorbusChris force-pushed the fcct-in-mc branch 2 times, most recently from a3a3dd6 to b9ad137 Compare August 6, 2020 17:03
@cgwalters cgwalters added the 4.7 Work deferred for 4.7 label Sep 3, 2020
@cgwalters
Copy link
Member

cgwalters commented Sep 3, 2020

I like the idea but I think we should do this after coreos/butane#79

Also MCO team: I added a 4.7 label we can use for PRs like this - "things to look at/merge after we branch for 4.6".

cgwalters
cgwalters requested changes Sep 3, 2020
View reviewed changes
Copy link
Member

@cgwalters cgwalters left a comment

Choose a reason for hiding this comment

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

Moving off my review queue for now

@LorbusChris
Copy link
Contributor Author

LorbusChris commented Sep 15, 2020

/retest

@LorbusChris
Copy link
Contributor Author

LorbusChris commented Oct 6, 2020

/hold cancel
/retest

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 6, 2020
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Oct 6, 2020 •
edited
Loading

@LorbusChris: The following test failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
ci/prow/e2e-aws-workers-rhel7 259fff40ef67dfba10a3d644e275fbf845ffbbfb link /test e2e-aws-workers-rhel7

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.

@cgwalters
Copy link
Member

cgwalters commented Oct 19, 2020

So we could probably land this one pretty quickly if we agree to conditionalize it first on being on FCOS/OKD right? There's low-to-zero risk for that and then we'd be able to tackle the "how does fcc work with RHCOS" as a separate phase or so.

@LorbusChris
Copy link
Contributor Author

LorbusChris commented Nov 3, 2020

I've created an enhancement proposal for this: openshift/enhancements#525

@cgwalters I'm not sure we need to conditionalize it for OKD - or are there any drawbacks to having it in OCP as a tech preview of sorts that is not used by any component by default, and not recommended to admin users yet?

@LorbusChris LorbusChris mentioned this pull request Nov 3, 2020
Closed
@LorbusChris
ctrlcommon/helpers: Update fcct to v0.7.0
063e172 
Updates FCCT so CoreOS configs can be transpiled to Ignition config spec v3.1
directly.
@LorbusChris
*: Support CoreOS configs in MCs
bb4c013 
This adds support for parsing CoreOS (FCCT) configuration
to the IgnParseWrapper function.

If a YAML-encoded CoreOS config is encountered, the MCC will
attempt to transpile and parse the data to Ignition config spec v3.1.
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Nov 23, 2020

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: LorbusChris
To complete the pull request process, please assign yuqi-zhang after the PR has been reviewed.
You can assign the PR to them by writing /assign @yuqi-zhang in a comment when ready.

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

Details 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

@kikisdeliveryservice
Copy link
Contributor

kikisdeliveryservice commented Nov 23, 2020

Agree on gating and also there's an open enhancement so it feels like that should be finished first..

/hold

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 23, 2020
@kikisdeliveryservice
Copy link
Contributor

kikisdeliveryservice commented Nov 23, 2020

Also: is this okd-e2e test dead?

@LorbusChris
Copy link
Contributor Author

LorbusChris commented Nov 23, 2020 •
edited
Loading

@kikisdeliveryservice I don't think master and 4.6 okd tests will succeed just, yet as machine-os-content for 4.6+ still has some issues, although it looks like it failed pulling busybox from docker just now (due to the new rate limiting their doing, I think at least that issue's been fixed).
/retest

@openshift-merge-robot
Copy link
Contributor

openshift-merge-robot commented Nov 23, 2020 •
edited
Loading

@LorbusChris: The following test failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
ci/prow/okd-e2e-aws bb4c013 link /test okd-e2e-aws

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.

@LorbusChris LorbusChris mentioned this pull request Dec 1, 2020
Merged
@openshift-bot
Copy link
Contributor

openshift-bot commented Feb 22, 2021

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

@openshift-ci-robot openshift-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Feb 22, 2021
@LorbusChris LorbusChris mentioned this pull request Mar 11, 2021
Merged
@openshift-bot
Copy link
Contributor

openshift-bot commented Mar 24, 2021

Stale issues rot after 30d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle rotten
/remove-lifecycle stale

@openshift-ci-robot openshift-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Mar 24, 2021
@LorbusChris LorbusChris closed this Apr 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cgwalters cgwalters cgwalters requested changes

@yuqi-zhang yuqi-zhang Awaiting requested review from yuqi-zhang

Assignees

No one assigned

Labels

4.7 Work deferred for 4.7 do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@LorbusChris @kikisdeliveryservice @cgwalters @openshift-ci-robot @openshift-merge-robot @openshift-bot