Codify performance manifests by cynepco3hahue · Pull Request #24 · openshift-kni/performance-addon-operators

cynepco3hahue · 2019-12-25T15:43:43Z

Codify performance manifests to make it easier to introduce complex logic for applying this manifest.

openshift-ci-robot · 2019-12-25T15:43:49Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cynepco3hahue

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:

~~OWNERS~~ [cynepco3hahue]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Signed-off-by: Artyom Lukianov <alukiano@redhat.com>

The asset directory will include: - shell scripts needed for performance machine configuration resource - tuned profiles needed for performance tuned resource Signed-off-by: Artyom Lukianov <alukiano@redhat.com>

yanirq · 2020-01-01T13:28:11Z

build/assets/scripts/pre-boot-tuning.sh

+
+    sed -i "s^initrd .*\$^& ${RHCOS_OSTREE_BOOTLOADER_PATH}iso_initrd.img^" $entry_file
+
+    #TODO - once RHCOS image contains the initrd content we can set parameters with rpm-ostree:


We might actually be able to test that already with latest RHCOS changes , but ill do it in another PR

- cluster feature gate resource - kubelet config resource - machine config resource - machine config pool - tuned resource Signed-off-by: Artyom Lukianov <alukiano@redhat.com>

It should validate correctness of yaml generation with specified parameters. Signed-off-by: Artyom Lukianov <alukiano@redhat.com>

yanirq · 2020-01-02T12:57:10Z

build/assets/tuned/openshift-node-network-latency

+[cpu]
+# https://github.com/redhat-performance/tuned/blob/master/profiles/latency-performance/tuned.conf
+# https://github.com/redhat-performance/tuned/blob/master/profiles/network-latency/tuned.conf
+force_latency=cstate.id:1|3


there might be an issue with this format of setting latency (using the OR operand) , investigating this but worth checking if tuned pod doesn't have any errors with that here .

We can fix it under the separate PR

yanirq · 2020-01-02T13:01:10Z

pkg/controller/performanceprofile/components/machineconfig/machineconfig.go

+}
+
+func getKernelArgs(hugePages *performancev1alpha1.HugePages, isolatedCPUs *performancev1alpha1.CPUSet) []string {
+	kargs := []string{


Can we pass these values in the CR as well ? Can we and do we want to pass then in the CR ?

Currently, I pass the whole performance profile.

yanirq · 2020-01-02T13:03:11Z

@cynepco3hahue overall looks ok , If there are specific files that were not migrated as they are and are worth a second look please specify them.
Also need to see e2e tests pass

Signed-off-by: Artyom Lukianov <alukiano@redhat.com>

cynepco3hahue · 2020-01-02T13:42:42Z

@yanirq e2e tests

@cynepco3hahue overall looks ok , If there are specific files that were not migrated as they are and are worth a second look please specify them.
Also need to see e2e tests pass

Fixed unittest, e2e tests requires the deployment of the operator, will create following PR once #25 will be merged.

yanirq · 2020-01-02T14:11:25Z

/lgtm

openshift-ci-robot · 2020-01-02T14:38:13Z

@cynepco3hahue: The following test failed, say /retest to rerun them all:

Test name	Commit	Details	Rerun command
ci/prow/e2e-gcp	`d29c154`	link	`/test e2e-gcp`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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.

davidvossel

nice pr! i know I'm reviewing retroactively. I just thought it would be worth leaving some notes of things I saw along the way of looking through the changes.

davidvossel · 2020-01-06T18:59:36Z

pkg/controller/performanceprofile/components/consts.go

+	// LabelMachineConfigurationRole defines the label for machine configuration role
+	LabelMachineConfigurationRole = "machineconfiguration.openshift.io/role"
+	// LableMachineConfigPoolRole defines the label for machine config pool role
+	LableMachineConfigPoolRole = "machineconfigpool.openshift.io/role"


s/lable/label

davidvossel · 2020-01-06T19:01:36Z

pkg/apis/performance/v1alpha1/performanceprofile_types.go

-	// Enabled enables real time kernel on relevant nodes.
-	Enabled *bool `json:"enabled,omitempty"`
+	// RepoURL defines the URL to the repository with real time kernel packages
+	RepoURL *string `json:"repoURL,omitempty"`


is this going to eventually turn back into a boolean soon?

davidvossel · 2020-01-06T19:20:39Z

pkg/controller/performanceprofile/components/tuned/tuned_test.go

+`
+
+var _ = Describe("Tuned", func() {
+	Context("with worker real time kerbnel profile", func() {


kerbnel/kernel

Added more tests to sctp feature. Tests: KernelModule, Connectivity

openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 25, 2019

openshift-ci-robot requested review from davidvossel and yanirq December 25, 2019 15:43

openshift-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Dec 25, 2019

Artyom Lukianov added 2 commits December 26, 2019 16:33

Update vendor dependencies

4ec0245

Signed-off-by: Artyom Lukianov <alukiano@redhat.com>

Add asset directory to the operator image

0c0f3bb

The asset directory will include: - shell scripts needed for performance machine configuration resource - tuned profiles needed for performance tuned resource Signed-off-by: Artyom Lukianov <alukiano@redhat.com>

cynepco3hahue force-pushed the create_resources branch from 0582bed to 02ce4dc Compare December 26, 2019 14:34

openshift-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Dec 26, 2019

cynepco3hahue force-pushed the create_resources branch from 02ce4dc to 59039db Compare December 29, 2019 08:52

cynepco3hahue changed the title ~~[WIP] Codify performance manifests~~ Codify performance manifests Dec 29, 2019

openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 29, 2019

cynepco3hahue force-pushed the create_resources branch 5 times, most recently from e368b48 to 5a3c2af Compare December 31, 2019 11:19

yanirq reviewed Jan 1, 2020

View reviewed changes

Artyom Lukianov added 2 commits January 2, 2020 11:19

Codify performance manifests

113478e

- cluster feature gate resource - kubelet config resource - machine config resource - machine config pool - tuned resource Signed-off-by: Artyom Lukianov <alukiano@redhat.com>

Add unit tests for all resources

f7c3a68

It should validate correctness of yaml generation with specified parameters. Signed-off-by: Artyom Lukianov <alukiano@redhat.com>

cynepco3hahue force-pushed the create_resources branch from 5a3c2af to f7c3a68 Compare January 2, 2020 09:20

yanirq reviewed Jan 2, 2020

View reviewed changes

Add real-time kernel repository field to API

d29c154

Signed-off-by: Artyom Lukianov <alukiano@redhat.com>

openshift-ci-robot assigned yanirq Jan 2, 2020

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jan 2, 2020

openshift-merge-robot merged commit 2291ac2 into openshift-kni:master Jan 2, 2020

davidvossel reviewed Jan 6, 2020

View reviewed changes

dshchedr pushed a commit to dshchedr/performance-addon-operators that referenced this pull request Mar 26, 2020

Merge pull request openshift-kni#24 from nikita133/sctp_tests

885dbb3

Added more tests to sctp feature. Tests: KernelModule, Connectivity


		sed -i "s^initrd .*\$^& ${RHCOS_OSTREE_BOOTLOADER_PATH}iso_initrd.img^" $entry_file

		#TODO - once RHCOS image contains the initrd content we can set parameters with rpm-ostree:

Conversation

cynepco3hahue commented Dec 25, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci-robot commented Dec 25, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yanirq commented Jan 2, 2020

Uh oh!

cynepco3hahue commented Jan 2, 2020

Uh oh!

yanirq commented Jan 2, 2020

Uh oh!

openshift-ci-robot commented Jan 2, 2020

Uh oh!

davidvossel left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

cynepco3hahue commented Dec 25, 2019 •

edited

Loading