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

Use Ignition 3 for MachineConfigs. #147

Merged
merged 1 commit into from Jul 27, 2020

Conversation

jmencak
Copy link
Contributor

@jmencak jmencak commented Jul 22, 2020

MCO in OCP 4.6 will use Ignition 3 by default. Follow the same path.

MCO in OCP 4.6 will use Ignition 3 by default.  Follow the same path.
@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 22, 2020
@jmencak
Copy link
Contributor Author

jmencak commented Jul 22, 2020

/test e2e-aws

@jmencak
Copy link
Contributor Author

jmencak commented Jul 23, 2020

@sjug / @kpouget PTAL
Thanks!

Copy link

@kpouget kpouget left a comment

Choose a reason for hiding this comment

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

the patch changes the Ignition import from github.com/coreos/ignition/config/v2_2/types to github.com/coreos/ignition/v2/config/v3_1/types, with the side effect that the 'ignition types configuration' needs to be passed raw (json) instead of a igntypes.Config object. Looks safe to me, the marshalization indeed should never fail.

The rest of the modifications is side effects of the package import updates.

Thanks for the PR Jiri

@jmencak
Copy link
Contributor Author

jmencak commented Jul 26, 2020

Thank you Kevin, much appreciated. Any chance of a lgtm label if you feel this is OK?

@kpouget
Copy link

kpouget commented Jul 26, 2020

/lgtm

@jmencak
Copy link
Contributor Author

jmencak commented Jul 26, 2020

Don't think bot picks up edits. But thanks!

/lgtm
^ based on message above.

@openshift-ci-robot
Copy link
Contributor

@jmencak: you cannot LGTM your own PR.

In response to this:

Don't think bot picks up edits. But thanks!

/lgtm
^ based on message above.

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.

@jmencak
Copy link
Contributor Author

jmencak commented Jul 26, 2020

Nice, it used to be possible to /lgtm own PRs. Good to know!

@kpouget
Copy link

kpouget commented Jul 26, 2020

/lgtm

I'm always bad with these IRC-like messages :# :# :# ;-)

and I think I already approved the PR when I validated my review:

kpouget approved these changes 4 days ago

but we'll see if this lgtm changes something or not

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 26, 2020
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jmencak, kpouget

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

@jmencak
Copy link
Contributor Author

jmencak commented Jul 26, 2020

but we'll see if this lgtm changes something or not

It does, many thanks! Time to pray for CI tests to pass again.

@kpouget
Copy link

kpouget commented Jul 26, 2020

but we'll see if this lgtm changes something or not

It does, many thanks! Time to pray for CI tests to pass again.

great, so "lgtm" and "approved" tags are not the same, I'll know it for next time :)

@jmencak
Copy link
Contributor Author

jmencak commented Jul 26, 2020

great, so "lgtm" and "approved" tags are not the same, I'll know it for next time :)

They're not and only folk listed as approvers can add an "approved" label to a PR.

@jmencak
Copy link
Contributor Author

jmencak commented Jul 26, 2020

/test e2e-aws

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

9 similar comments
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

13 similar comments
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-merge-robot openshift-merge-robot merged commit c7e9ccf into openshift:master Jul 27, 2020
@jmencak jmencak deleted the 4.6-ignition-3.1.0 branch July 28, 2020 05:19
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. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants