Skip to content

Conversation

@harche
Copy link
Contributor

@harche harche commented Oct 13, 2022

Signed-off-by: Harshal Patil harpatil@redhat.com

Fixes: https://issues.redhat.com/browse/OCPBUGS-2079

- What I did
Add missing ephemeral storage value to system reserved arg of the kubelet

- How to verify it
Log into the node, and verify that kubelet process has started with the system reserved arg that has ephemeral storage

- Description for the changelog

@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 13, 2022
@openshift-ci openshift-ci bot requested review from cgwalters and jkyros October 13, 2022 09:29
Signed-off-by: Harshal Patil <harpatil@redhat.com>
@harche harche changed the title WIP: Add ephemeral storage to kubelet system reserved args Add ephemeral storage to kubelet system reserved args Oct 17, 2022
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 17, 2022
@harche harche changed the title Add ephemeral storage to kubelet system reserved args OCPBUGS-2079: Add ephemeral storage to kubelet system reserved args Oct 17, 2022
@openshift-ci-robot openshift-ci-robot added the jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. label Oct 17, 2022
@openshift-ci-robot
Copy link
Contributor

@harche: This pull request references Jira Issue OCPBUGS-2079, which is invalid:

  • expected the bug to target the "4.12.0" version, but no target version was set

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

The bug has been updated to refer to the pull request using the external bug tracker.

Details

In response to this:

Signed-off-by: Harshal Patil harpatil@redhat.com

Fixes: https://issues.redhat.com/browse/OCPBUGS-2079

- What I did
Add missing ephemeral storage value to system reserved arg of the kubelet

- How to verify it
Log into the node, and verify that kubelet process has started with the system reserved arg that has ephemeral storage

- Description for the changelog

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.

@harche
Copy link
Contributor Author

harche commented Oct 17, 2022

/cc @rphillips @QiWang19

@openshift-ci openshift-ci bot requested review from QiWang19 and rphillips October 17, 2022 08:01
@harche
Copy link
Contributor Author

harche commented Oct 17, 2022

/jira refresh

@openshift-ci-robot openshift-ci-robot added jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. and removed jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. labels Oct 17, 2022
@openshift-ci-robot
Copy link
Contributor

@harche: This pull request references Jira Issue OCPBUGS-2079, which is valid.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target version (4.12.0) matches configured target version for branch (4.12.0)
  • bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST)

No GitHub users were found matching the public email listed for the QA contact in Jira (schoudha@redhat.com), skipping review request.

Details

In response to this:

/jira refresh

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.

Copy link
Member

@QiWang19 QiWang19 left a comment

Choose a reason for hiding this comment

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

LGTM

@harche
Copy link
Contributor Author

harche commented Oct 18, 2022

/retest-required

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 18, 2022

@harche: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/okd-scos-e2e-aws 28f062b link false /test okd-scos-e2e-aws
ci/prow/e2e-openstack 28f062b link false /test e2e-openstack

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.

@rphillips
Copy link
Contributor

/lgtm
/approve
/retest-required

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Oct 18, 2022
@rphillips
Copy link
Contributor

/assign @yuqi-zhang

Copy link
Contributor

@yuqi-zhang yuqi-zhang left a comment

Choose a reason for hiding this comment

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

lgtm. I believe the function is used for both bootstrap and in-cluster, so we should be covered there, but out of curiosity, did you test bootstrapping with this kubeletconfig?

@harche
Copy link
Contributor Author

harche commented Oct 19, 2022

lgtm. I believe the function is used for both bootstrap and in-cluster, so we should be covered there, but out of curiosity, did you test bootstrapping with this kubeletconfig?

I launched a cluster using cluster-bot with this PR, and the functionality worked out of box in that case.

@yuqi-zhang
Copy link
Contributor

I launched a cluster using cluster-bot with this PR, and the functionality worked out of box in that case.

Right, I meant launch a PR with a user-provided kubeletconfig as part of the bootstrap manifests that sets ephemeral storage, which I didn't know you could do via cluster-bot, but I could just be completely out of date

@harche
Copy link
Contributor Author

harche commented Oct 19, 2022

I launched a cluster using cluster-bot with this PR, and the functionality worked out of box in that case.

Right, I meant launch a PR with a user-provided kubeletconfig as part of the bootstrap manifests that sets ephemeral storage, which I didn't know you could do via cluster-bot, but I could just be completely out of date

Oh ok, got you now. Let me test it that way. Thanks.

@yuqi-zhang
Copy link
Contributor

Strange, I approved the PR but the approved label never applied. Let me know how that goes, and I'll try again tomorrow

@harche
Copy link
Contributor Author

harche commented Oct 20, 2022

Strange, I approved the PR but the approved label never applied. Let me know how that goes, and I'll try again tomorrow

@yuqi-zhang I tested in the bootstrap mode but it seems all values of system reserved (i.e. cpu, memory, ephemeral storage) are dropped. We will have to re-test the changes from #2547

I used following KubeletConfig to drop in manifest folder,

apiVersion: machineconfiguration.openshift.io/v1
kind: KubeletConfig
metadata:
  name: system-reserved-config2
spec:
  machineConfigPoolSelector:
    matchLabels:
      pools.operator.machineconfiguration.openshift.io/worker: ""
  kubeletConfig:
    systemReserved:
      cpu: 500m
      memory: 500Mi
      ephemeral-storage: 15Gi

The kubelet came up with all default values for memory,cpu, ephemeral storage,

--system-reserved=cpu=500m,memory=1Gi,ephemeral-storage=1Gi

The expected values were (without the change in this PR the value for ephemeral storage won't change anyway),

 --system-reserved=cpu=500m,memory=500Mi,ephemeral-storage=1Gi

I have created a separate bug to track this issue with bootstrap mode and system reserved, https://issues.redhat.com/browse/OCPBUGS-2634

cc @QiWang19 @rphillips

@harche
Copy link
Contributor Author

harche commented Oct 20, 2022

Hold on, I might have made some silly mistake while testing. I will retest and report back here.

@harche
Copy link
Contributor Author

harche commented Oct 20, 2022

By mistake I used the wrong cluster for testing. I re-ran the tests using the proper cluster and it seems to be working fine. Sorry for the noise. The value of the ephemeral storage in system reserved was correctly applied in kubelet args (in bootstrap mode),

--system-reserved=cpu=500m,memory=500Mi,ephemeral-storage=15Gi 

cc @yuqi-zhang

@yuqi-zhang
Copy link
Contributor

ack, thanks for the additional testing!

/approve

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 20, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: harche, rphillips, yuqi-zhang

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 openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 20, 2022
@openshift-merge-robot openshift-merge-robot merged commit 27b5e69 into openshift:master Oct 20, 2022
@openshift-ci-robot
Copy link
Contributor

@harche: All pull requests linked via external trackers have merged:

Jira Issue OCPBUGS-2079 has been moved to the MODIFIED state.

Details

In response to this:

Signed-off-by: Harshal Patil harpatil@redhat.com

Fixes: https://issues.redhat.com/browse/OCPBUGS-2079

- What I did
Add missing ephemeral storage value to system reserved arg of the kubelet

- How to verify it
Log into the node, and verify that kubelet process has started with the system reserved arg that has ephemeral storage

- Description for the changelog

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.

@harche
Copy link
Contributor Author

harche commented Oct 21, 2022

/cherry-pick release-4.11

@openshift-cherrypick-robot

@harche: new pull request created: #3381

Details

In response to this:

/cherry-pick release-4.11

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.

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. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. 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