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

dind: ensure node certs are generated serially #11298

Closed
wants to merge 1 commit into from

Conversation

marun
Copy link
Contributor

@marun marun commented Oct 10, 2016

The cert gen commands are not intended to be run concurrently.

Intended to fix #11274

cc: @liggitt @openshift/networking @stevekuznetsov

@marun
Copy link
Contributor Author

marun commented Oct 10, 2016

[test]

@marun
Copy link
Contributor Author

marun commented Oct 10, 2016

flakes #11094 #9490 #9548

re-[test]


exec 200>"${config_path}"/.openshift-generate-node-config.exclusivelock

flock -n 200 && return 0 || return 1
Copy link
Contributor

Choose a reason for hiding this comment

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

What does masking the return code here achieve? The only difference is non-zero return codes get aliased as 1.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@@ -8,6 +8,14 @@ source /usr/local/bin/openshift-dind-lib.sh
# Should set OPENSHIFT_NETWORK_PLUGIN
source /data/network-plugin

function lock() {
Copy link
Contributor

Choose a reason for hiding this comment

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

We probably want to namespace this function, I feel like we could have name collisions with lock

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@marun
Copy link
Contributor Author

marun commented Oct 11, 2016

flake #11240

re-[test]

@marun
Copy link
Contributor Author

marun commented Oct 11, 2016

@stevekuznetsov ptal

@stevekuznetsov
Copy link
Contributor

LGTM

@danwinship
Copy link
Contributor

Shouldn't we drop the lock at some point? AIUI, once we get the lock, this would hold it until the script exits

Copy link
Contributor

@knobunc knobunc left a comment

Choose a reason for hiding this comment

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

LGTM

@marun
Copy link
Contributor Author

marun commented Oct 11, 2016

@danwinship I'm not sure it's a problem to hold it until the file exits, but it can't hurt to unlock after config is generated.

The cert gen commands are not intended to be run concurrently.
@openshift-bot
Copy link
Contributor

Evaluated for origin test up to 5efe73a

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/test Running (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/9917/)

@marun
Copy link
Contributor Author

marun commented Oct 11, 2016

Closed in favor of inclusion in #11326.

@marun marun closed this Oct 11, 2016
@marun marun deleted the dind-sync-cert-gen branch November 29, 2016 19:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

networking test flake "certificate is valid for nettest-node-1, 172.17.0.3, not nettest-node-2"
5 participants