Skip to content

Update kernel args on firstboot-complete#1942

Merged
openshift-merge-robot merged 1 commit intoopenshift:masterfrom
vrutkovs:kargs-on-firstboot
Jul 24, 2020
Merged

Update kernel args on firstboot-complete#1942
openshift-merge-robot merged 1 commit intoopenshift:masterfrom
vrutkovs:kargs-on-firstboot

Conversation

@vrutkovs
Copy link
Copy Markdown
Contributor

@vrutkovs vrutkovs commented Jul 22, 2020

This commits ensures kernel args modifications from /etc/pivot/kernel-args
are being applied on both pivot and firstboot_complete_machineconfig. Currently its only applied on bootstrap (which uses pivot), but not on masters/workers. In order to archieve that kargs tuning logic is moved from cmd/ package to pkg/daemon.

OKD requires this so that "mitigations" karg could be removed from cmdline,
as MachineConfig's kernelArguments doesn't support removing existing arguments. This should also affect OCP with HyperThreading off, as machineconfigs for that still use /etc/pivot/kernel-args

@vrutkovs
Copy link
Copy Markdown
Contributor Author

/cc @LorbusChris @sinnykumari

This commits ensures kernel args modifications from /etc/pivot/kernel-args
are being applied on both pivot and firstboot_complete_machineconfig.

OKD requires this so that "mitigations" karg could be removed from cmdline,
as MachineConfig's `kernelArguments` doesn't support removing existing arguments.
@vrutkovs vrutkovs force-pushed the kargs-on-firstboot branch from e00db5e to 8eb1010 Compare July 22, 2020 11:07
@LorbusChris
Copy link
Copy Markdown
Contributor

/approve

@cgwalters
Copy link
Copy Markdown
Member

This is strongly related to #1925

OK right, I understand this now.

I think what would be a lot simpler and get us away from the /etc/pivot stuff is to just hardcode in the MCD:

if os == FCOS {
  run(rpm-ostree kargs --remove mitigations=auto,nosmt)
}

But for now
/approve
/lgtm

@openshift-ci-robot
Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cgwalters, LorbusChris, vrutkovs

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

The pull request process is described 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

@openshift-ci-robot openshift-ci-robot added lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jul 24, 2020
@vrutkovs
Copy link
Copy Markdown
Contributor Author

I think what would be a lot simpler and get us away from the /etc/pivot stuff is to just hardcode in the MCD

We'd need to include hyperthreading off case for RHCOS too (its not widely used though)

@openshift-bot
Copy link
Copy Markdown
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 be70bfe into openshift:master Jul 24, 2020
Comment thread pkg/daemon/daemon.go
return errors.Wrap(err, "failed to rename encapsulated MachineConfig after processing on firstboot")
}

tuningChanged, err := UpdateTuningArgs(KernelTuningFile, CmdLineFile)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

A bit late to see this PR. Having this functionality here is going to work only during firstboot. Isn't it that users who maybe using this functionality would want to add/delete these kargs later on as well? Since, these are not applied using MachineConfig, there is no standard way to undo changes done during firstboot.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Right, removing default kargs via MCs should be solved in https://issues.redhat.com/browse/GRPA-2490

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@vrutkovs vrutkovs deleted the kargs-on-firstboot branch September 16, 2020 22:29
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.

7 participants