Bug 1874696: Remove systemctl calls#785
Merged
openshift-merge-robot merged 1 commit intoopenshift:masterfrom Sep 30, 2020
Merged
Bug 1874696: Remove systemctl calls#785openshift-merge-robot merged 1 commit intoopenshift:masterfrom
openshift-merge-robot merged 1 commit intoopenshift:masterfrom
Conversation
pliurh
reviewed
Sep 10, 2020
Contributor
pliurh
left a comment
pliurh
left a comment
There was a problem hiding this comment.
After fixing the nits, It works as expected in my environment.
pliurh
reviewed
Sep 10, 2020
pliurh
reviewed
Sep 10, 2020
pliurh
reviewed
Sep 10, 2020
juanluisvaladas
force-pushed
the
replace_systemctl
branch
from
0975174 to
8f32c79
Compare
Contributor
|
/lgtm |
Contributor
|
/assign @knobunc |
Contributor
|
@juanluisvaladas Could you put the bz id 1874696 in the title? |
Contributor
Author
|
/retitle Bug 1874696: Replace systemctl calls with DBus API calls |
openshift-ci-robot
changed the title
openshift-ci-robot
added
bugzilla/severity-high
Contributor
|
@juanluisvaladas: This pull request references Bugzilla bug 1874696, which is valid. 3 validation(s) were run on this bug
DetailsIn response to this:
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. |
Contributor
Author
|
/retest |
2 similar comments
Contributor
Author
|
/retest |
Contributor
Author
|
/retest |
Contributor
|
/lgtm |
openshift-ci-robot
added
the
approved
Contributor
|
/retest Please review the full test history for this PR and help us cut down flakes. |
5 similar comments
Contributor
|
/retest Please review the full test history for this PR and help us cut down flakes. |
Contributor
|
/retest Please review the full test history for this PR and help us cut down flakes. |
Contributor
|
/retest Please review the full test history for this PR and help us cut down flakes. |
Contributor
|
/retest Please review the full test history for this PR and help us cut down flakes. |
Contributor
|
/retest Please review the full test history for this PR and help us cut down flakes. |
Contributor
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: abhat, danielmellado, danwinship, juanluisvaladas, pliurh The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Contributor
|
/hold The upgrade job was for ovn. Need sdn upgrade passing as well. |
Contributor
|
/hold |
Contributor
Author
|
/hold cancel |
openshift-ci-robot
removed
the
do-not-merge/hold
Contributor
|
/retest Please review the full test history for this PR and help us cut down flakes. |
1 similar comment
Contributor
|
/retest Please review the full test history for this PR and help us cut down flakes. |
Contributor
|
Upgrade jobs succeeded from 4.6 to 4.6 this time on SDN: https://prow.ci.openshift.org/view/gs/origin-ci-test/logs/release-openshift-origin-installer-launch-aws/1311054422679228416 |
Contributor
|
/retest Please review the full test history for this PR and help us cut down flakes. |
5 similar comments
Contributor
|
/retest Please review the full test history for this PR and help us cut down flakes. |
Contributor
|
/retest Please review the full test history for this PR and help us cut down flakes. |
Contributor
|
/retest Please review the full test history for this PR and help us cut down flakes. |
Contributor
|
/retest Please review the full test history for this PR and help us cut down flakes. |
Contributor
|
/retest Please review the full test history for this PR and help us cut down flakes. |
Contributor
|
@knobunc, need another override on the e2e-upgrade job. |
Contributor
|
/override ci/prow/e2e-upgrade |
Contributor
|
@trozet: Overrode contexts on behalf of trozet: ci/prow/e2e-upgrade DetailsIn response to this:
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. |
Contributor
|
@juanluisvaladas: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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. |
Contributor
|
/retest Please review the full test history for this PR and help us cut down flakes. |
Contributor
|
@juanluisvaladas: All pull requests linked via external trackers have merged: Bugzilla bug 1874696 has been moved to the MODIFIED state. DetailsIn response to this:
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. |
vrutkovs
pushed a commit
to vrutkovs/ci-tools
that referenced
this pull request
Oct 2, 2020
…iodSeconds Sometimes test steps like updates take a long time [1]. When that happens, something wrapping the workflow may be timed out instead, and the whole thing gets torn down without any post-test gather/teardown steps running. With this commit, we expose two timeout-related properties from PodSpec [2] to step maintainers to use to control this behavior. Step maintainers can now set per-step timeouts that are comfortably less than the wrapper timeout, ensuring that slow/hung steps get killed, and the remaining steps have some time to gather details that will explain the delay and perform other teardown. I personally prefer matching the casing of the wrapped Kubernetes properties, but Bruno and Petr prefer matching the snake_casing of the existing step-config properties [3], so that's what we have in this commit. [1]: openshift/cluster-network-operator#785 (comment) [2]: https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.18/#podspec-v1-core [3]: openshift#1257 (comment)
vrutkovs
pushed a commit
to vrutkovs/ci-tools
that referenced
this pull request
Oct 3, 2020
…iodSeconds Sometimes test steps like updates take a long time [1]. When that happens, something wrapping the workflow may be timed out instead, and the whole thing gets torn down without any post-test gather/teardown steps running. With this commit, we expose two timeout-related properties from PodSpec [2] to step maintainers to use to control this behavior. Step maintainers can now set per-step timeouts that are comfortably less than the wrapper timeout, ensuring that slow/hung steps get killed, and the remaining steps have some time to gather details that will explain the delay and perform other teardown. I personally prefer matching the casing of the wrapped Kubernetes properties, but Bruno and Petr prefer matching the snake_casing of the existing step-config properties [3], so that's what we have in this commit. [1]: openshift/cluster-network-operator#785 (comment) [2]: https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.18/#podspec-v1-core [3]: openshift#1257 (comment)
wking
added a commit
to wking/ci-tools
that referenced
this pull request
Oct 5, 2020
…iodSeconds Sometimes test steps like updates take a long time [1]. When that happens, something wrapping the workflow may be timed out instead, and the whole thing gets torn down without any post-test gather/teardown steps running. With this commit, we expose two timeout-related properties from PodSpec [2] to step maintainers to use to control this behavior. Step maintainers can now set per-step timeouts that are comfortably less than the wrapper timeout, ensuring that slow/hung steps get killed, and the remaining steps have some time to gather details that will explain the delay and perform other teardown. I personally prefer matching the casing of the wrapped Kubernetes properties, but Bruno and Petr prefer matching the snake_casing of the existing step-config properties [3], so that's what we have in this commit. It seems like we'd want these to be configurable alongside all the places where 'commands' was configurable. But: * I dunno what PipelineImageCacheStepConfiguration is about, so I did not add these properties there. * Steve says he doesn't want them on TestStepConfiguration [4], which handles the 'tests' section of the configuration, including container, template, multi-step tests [5]. So I'm only adding them to the single-step LiteralTestStep. [1]: openshift/cluster-network-operator#785 (comment) [2]: https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.18/#podspec-v1-core [3]: openshift#1257 (comment) [4]: openshift#1257 (comment) [5]: openshift#1257 (comment)
wking
added a commit
to wking/ci-tools
that referenced
this pull request
Oct 5, 2020
…iodSeconds Sometimes test steps like updates take a long time [1]. When that happens, something wrapping the workflow may be timed out instead, and the whole thing gets torn down without any post-test gather/teardown steps running. With this commit, we expose two timeout-related properties from PodSpec [2] to step maintainers to use to control this behavior. Step maintainers can now set per-step timeouts that are comfortably less than the wrapper timeout, ensuring that slow/hung steps get killed, and the remaining steps have some time to gather details that will explain the delay and perform other teardown. I personally prefer matching the casing of the wrapped Kubernetes properties, but Bruno and Petr prefer matching the snake_casing of the existing step-config properties [3], so that's what we have in this commit. It seems like we'd want these to be configurable alongside all the places where 'commands' was configurable. But: * I dunno what PipelineImageCacheStepConfiguration is about, so I did not add these properties there. * Steve says he doesn't want them on TestStepConfiguration [4], which handles the 'tests' section of the configuration, including container, template, multi-step tests [5]. So I'm only adding them to the single-step LiteralTestStep. [1]: openshift/cluster-network-operator#785 (comment) [2]: https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.18/#podspec-v1-core [3]: openshift#1257 (comment) [4]: openshift#1257 (comment) [5]: openshift#1257 (comment)
vrutkovs
pushed a commit
to vrutkovs/ci-tools
that referenced
this pull request
Oct 6, 2020
…iodSeconds Sometimes test steps like updates take a long time [1]. When that happens, something wrapping the workflow may be timed out instead, and the whole thing gets torn down without any post-test gather/teardown steps running. With this commit, we expose two timeout-related properties from PodSpec [2] to step maintainers to use to control this behavior. Step maintainers can now set per-step timeouts that are comfortably less than the wrapper timeout, ensuring that slow/hung steps get killed, and the remaining steps have some time to gather details that will explain the delay and perform other teardown. I personally prefer matching the casing of the wrapped Kubernetes properties, but Bruno and Petr prefer matching the snake_casing of the existing step-config properties [3], so that's what we have in this commit. [1]: openshift/cluster-network-operator#785 (comment) [2]: https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.18/#podspec-v1-core [3]: openshift#1257 (comment)
wking
added a commit
to wking/ci-tools
that referenced
this pull request
Oct 7, 2020
…iodSeconds Sometimes test steps like updates take a long time [1]. When that happens, something wrapping the workflow may be timed out instead, and the whole thing gets torn down without any post-test gather/teardown steps running. With this commit, we expose two timeout-related properties from PodSpec [2] to step maintainers to use to control this behavior. Step maintainers can now set per-step timeouts that are comfortably less than the wrapper timeout, ensuring that slow/hung steps get killed, and the remaining steps have some time to gather details that will explain the delay and perform other teardown. I personally prefer matching the casing of the wrapped Kubernetes properties, but Bruno and Petr prefer matching the snake_casing of the existing step-config properties [3], so that's what we have in this commit. It seems like we'd want these to be configurable alongside all the places where 'commands' was configurable. But: * I dunno what PipelineImageCacheStepConfiguration is about, so I did not add these properties there. * Steve says he doesn't want them on TestStepConfiguration [4], which handles the 'tests' section of the configuration, including container, template, multi-step tests [5]. So I'm only adding them to the single-step LiteralTestStep. [1]: openshift/cluster-network-operator#785 (comment) [2]: https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.18/#podspec-v1-core [3]: openshift#1257 (comment) [4]: openshift#1257 (comment) [5]: openshift#1257 (comment)
wking
added a commit
to wking/ci-tools
that referenced
this pull request
Oct 7, 2020
…iodSeconds Sometimes test steps like updates take a long time [1]. When that happens, something wrapping the workflow may be timed out instead, and the whole thing gets torn down without any post-test gather/teardown steps running. With this commit, we expose two timeout-related properties from PodSpec [2] to step maintainers to use to control this behavior. Step maintainers can now set per-step timeouts that are comfortably less than the wrapper timeout, ensuring that slow/hung steps get killed, and the remaining steps have some time to gather details that will explain the delay and perform other teardown. I personally prefer matching the casing of the wrapped Kubernetes properties, but Bruno and Petr prefer matching the snake_casing of the existing step-config properties [3], so that's what we have in this commit. It seems like we'd want these to be configurable alongside all the places where 'commands' was configurable. But: * I dunno what PipelineImageCacheStepConfiguration is about, so I did not add these properties there. * Steve says he doesn't want them on TestStepConfiguration [4], which handles the 'tests' section of the configuration, including container, template, multi-step tests [5]. So I'm only adding them to the single-step LiteralTestStep. [1]: openshift/cluster-network-operator#785 (comment) [2]: https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.18/#podspec-v1-core [3]: openshift#1257 (comment) [4]: openshift#1257 (comment) [5]: openshift#1257 (comment)
wking
added a commit
to wking/ci-tools
that referenced
this pull request
Oct 8, 2020
…iodSeconds Sometimes test steps like updates take a long time [1]. When that happens, something wrapping the workflow may be timed out instead, and the whole thing gets torn down without any post-test gather/teardown steps running. With this commit, we expose two timeout-related properties from PodSpec [2] to step maintainers to use to control this behavior. Step maintainers can now set per-step timeouts that are comfortably less than the wrapper timeout, ensuring that slow/hung steps get killed, and the remaining steps have some time to gather details that will explain the delay and perform other teardown. I personally prefer matching the casing of the wrapped Kubernetes properties, but Bruno and Petr prefer matching the snake_casing of the existing step-config properties [3], so that's what we have in this commit. It seems like we'd want these to be configurable alongside all the places where 'commands' was configurable. But: * I dunno what PipelineImageCacheStepConfiguration is about, so I did not add these properties there. * Steve says he doesn't want them on TestStepConfiguration [4], which handles the 'tests' section of the configuration, including container, template, multi-step tests [5]. So I'm only adding them to the single-step LiteralTestStep. [1]: openshift/cluster-network-operator#785 (comment) [2]: https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.18/#podspec-v1-core [3]: openshift#1257 (comment) [4]: openshift#1257 (comment) [5]: openshift#1257 (comment)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
14 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
/assign @pliurh
When systemctl is run in a separate PID namespace where PID 1 isn't systemd, it doesn't behave the normal way: systemd/systemd#11300
The solution for this is to entirely remove calls to systemctl calls.