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

Update configure-helper.sh #104135

Merged
merged 3 commits into from
Aug 4, 2021
Merged

Update configure-helper.sh #104135

merged 3 commits into from
Aug 4, 2021

Conversation

vteratipally
Copy link
Contributor

add live-restore true by default.

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?


Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:


add live-restore true by default.
@k8s-ci-robot k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/needs-kind Indicates a PR lacks a `kind/foo` label and requires one. do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. area/provider/gcp Issues or PRs related to gcp provider labels Aug 4, 2021
@k8s-ci-robot k8s-ci-robot requested review from karan and roycaihw August 4, 2021 19:09
@k8s-ci-robot k8s-ci-robot added needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. sig/cloud-provider Categorizes an issue or PR as relevant to SIG Cloud Provider. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Aug 4, 2021
@@ -1506,12 +1506,6 @@ function set_docker_options_non_ubuntu() {

addockeropt "\"mtu\": 1460,"
Copy link
Member

Choose a reason for hiding this comment

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

these should also be moved a level up then? Since the config file is overrriden, it doesn't matter what were the default any longer

Copy link
Contributor Author

Choose a reason for hiding this comment

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

live-restore and storage-driver is already as an input in the ubuntu preloading image. Adding these to daemon.json might result in failure.

@cheftako
Copy link
Member

cheftako commented Aug 4, 2021

/sig cloud-provider
/sig node

@k8s-ci-robot k8s-ci-robot added sig/node Categorizes an issue or PR as relevant to SIG Node. and removed do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Aug 4, 2021
@cheftako
Copy link
Member

cheftako commented Aug 4, 2021

/release-note-none

@k8s-ci-robot k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Aug 4, 2021
@k8s-ci-robot k8s-ci-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Aug 4, 2021
@vteratipally
Copy link
Contributor Author

/retest

echo "setting live restore"
# Disable live-restore if the environment variable is set.
if [[ "${DISABLE_DOCKER_LIVE_RESTORE:-false}" == "true" ]]; then
addockeropt "\"live-restore\": \"false\","
addockeropt "\"live-restore\": false,"
Copy link
Member

Choose a reason for hiding this comment

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

are quotes removed intentionally?

Copy link
Member

Choose a reason for hiding this comment

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

checked, likely not-quoted version works better

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, setting the string for boolean values is not working properly.

@SergeyKanzhelev
Copy link
Member

SergeyKanzhelev commented Aug 4, 2021

/lgtm
/kind bug
/priority important-soon
/triage accepted

marking as a bug as when this is missing, the behavior on docker restart is very disruptive. It is a degradation from the logic we had before switching to using docker config instead of command line options

@k8s-ci-robot k8s-ci-robot added kind/bug Categorizes issue or PR as related to a bug. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. triage/accepted Indicates an issue or PR is ready to be actively worked on. labels Aug 4, 2021
@k8s-ci-robot k8s-ci-robot removed do-not-merge/needs-kind Indicates a PR lacks a `kind/foo` label and requires one. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Aug 4, 2021
@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 4, 2021
@dchen1107
Copy link
Member

/lgtm
/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dchen1107, vteratipally

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 4, 2021
@k8s-ci-robot k8s-ci-robot merged commit afb8d23 into kubernetes:master Aug 4, 2021
@k8s-ci-robot k8s-ci-robot added this to the v1.23 milestone Aug 4, 2021
k8s-ci-robot added a commit that referenced this pull request Aug 6, 2021
…-of-#104135-upstream-release-1.20

Automated cherry pick of #104135: Update configure-helper.sh
k8s-ci-robot added a commit that referenced this pull request Aug 6, 2021
…-of-#104135-upstream-release-1.21

Automated cherry pick of #104135: Update configure-helper.sh
k8s-ci-robot added a commit that referenced this pull request Aug 6, 2021
…-of-#104135-upstream-release-1.22

Automated cherry pick of #104135: Update configure-helper.sh
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. area/provider/gcp Issues or PRs related to gcp provider cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/bug Categorizes issue or PR as related to a bug. lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. release-note-none Denotes a PR that doesn't merit a release note. sig/cloud-provider Categorizes an issue or PR as relevant to SIG Cloud Provider. sig/node Categorizes an issue or PR as relevant to SIG Node. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
Development

Successfully merging this pull request may close these issues.

5 participants