Add support for CRIO drop-in config files#1660
Merged
openshift-merge-robot merged 2 commits intoopenshift:masterfrom Apr 22, 2020
Merged
Add support for CRIO drop-in config files#1660openshift-merge-robot merged 2 commits intoopenshift:masterfrom
openshift-merge-robot merged 2 commits intoopenshift:masterfrom
Conversation
Member
|
yowzers that's a lot of files, we gotta remember to bump the vendor in 4.6 🙃 looks good to start, I'll do a more thorough review tomorrow 🎉 😄 |
runcom
reviewed
Apr 16, 2020
umohnani8
force-pushed
the
dropin-files
branch
8 times, most recently
from
ed69e88 to
7742abf
Compare
umohnani8
force-pushed
the
dropin-files
branch
2 times, most recently
from
a23d58e to
0a2f2db
Compare
Member
|
looks great @umohnani8 one more nit then LGTM 😄 |
Contributor
Author
kikisdeliveryservice
approved these changes
Apr 17, 2020
Contributor
kikisdeliveryservice
left a comment
kikisdeliveryservice
left a comment
There was a problem hiding this comment.
I noticed this before, but I think that your custom type called ignitionConfig is confusing and misnamed (seems like it should be dropinFiles or something. Not blocking, but should be fixed.
openshift-ci-robot
added
the
approved
mtrmac
reviewed
Apr 18, 2020
Contributor
|
(I have only reviewed the mechanism of the thing, not the substance of change to how CRI-O is configured.) |
runcom
reviewed
Apr 19, 2020
Member
|
This should be good now pending CI which hasn't cooperated so far, we'll figure out follow ups then - going to lgtm this as it's definitely done but we need to hear from CI, the lgtm will go away if the code needs changing ofc. /lgtm |
Contributor
|
/skip |
Contributor
|
/retest |
1 similar comment
Contributor
Author
|
/retest |
umohnani8
force-pushed
the
dropin-files
branch
2 times, most recently
from
5527e3e to
1cf769b
Compare
Member
|
/retest |
Member
|
/skip |
umohnani8
force-pushed
the
dropin-files
branch
2 times, most recently
from
9cd492a to
86350ca
Compare
mtrmac
reviewed
Apr 20, 2020
mtrmac
reviewed
Apr 20, 2020
CRI-O now has support for drop-in files in /etc/crio/crio.conf.d so add that functionality to the ctrcfg controller. This will help avoid the issues we run into when we make config changes in cri-o and don't carry them over to the MCO. Remove the crio.conf template and put the MCO overrides in 00-default under the /etc/crio/crio.conf.d directory. Signed-off-by: Urvashi Mohnani <umohnani@redhat.com>
Since we don't depend on cri-o anymore, we no longer need to vendor it. Update other vendor dependencies as well. Signed-off-by: Urvashi Mohnani <umohnani@redhat.com>
Member
|
/retest |
Member
|
@mrunalp I think this should be merged but we're past that date and it's now asking for a bugzilla, do you know how to proceed here? |
mrunalp
added
the
bugzilla/valid-bug
Member
|
/lgtm |
Contributor
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: kikisdeliveryservice, mrunalp, runcom, umohnani8 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
8 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
- What I did
CRI-O now has support for drop-in files in /etc/crio/crio.conf.d
so add that functionality to the ctrcfg controller.
This will help avoid the issues we run into when we make config
changes in cri-o and don't carry them over to the MCO.
Remove the crio.conf template and put the MCO overrides in 00-default
under the /etc/crio/crio.conf.d directory.
Also update the vendor as we no longer depend on cri-o.
- How to verify it
Create ctrcfg CR and the updates should be added to /etc/crio/crio.conf.d/01-newconfig.
- Description for the changelog
Signed-off-by: Urvashi Mohnani umohnani@redhat.com