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

v1.8 backport: ci: Disable NFS locking #16970

Merged
merged 1 commit into from
Jul 22, 2021

Conversation

kkourt
Copy link
Contributor

@kkourt kkourt commented Jul 22, 2021

The last 1.8 backporting round (#16912), skipped #16554 due to CI failures in the test-upstream-k8s pipeline.

This PR is a backport of just #16554.

Once this PR is merged, you can update the PR labels via:

$ contrib/backporting/set-labels.py 16554 done 1.8

[ upstream commit 1dd477d ]

This is an attempt to fix the recent issues with NFS locking in CI, e.g.
issue cilium#16551

From the nfs(5) manpage:

> When using the nolock option, applications can lock files, but such
> locks provide exclusion only against other applications running on
> the same client.  Remote applications are not affected by these locks.

Since in CI, we do not have any remote applications accessing the shared
folder, only using local locks should be safe and more robust than using
distributed locking.

Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
Signed-off-by: Kornilios Kourtis <kornilios@isovalent.com>
@kkourt kkourt requested a review from a team as a code owner July 22, 2021 09:14
@maintainer-s-little-helper maintainer-s-little-helper bot added backport/1.8 kind/backports This PR provides functionality previously merged into master. labels Jul 22, 2021
@kkourt kkourt requested a review from gandro July 22, 2021 09:15
@kkourt
Copy link
Contributor Author

kkourt commented Jul 22, 2021

test-backport-1.8

Copy link
Member

@gandro gandro left a comment

Choose a reason for hiding this comment

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

Looks good! Thanks for doing this!

@kkourt
Copy link
Contributor Author

kkourt commented Jul 22, 2021

The motivation here is that it is not clear whether #16554 actually causes issues with VM provisioning. If there are no such issues in this PR, then it might be unrelated. Otherwise, we can investigate further.

@kkourt
Copy link
Contributor Author

kkourt commented Jul 22, 2021

k8s-upstream (test-upstream-k8s) failed again, but I think this is a different failure: https://jenkins.cilium.io/job/Cilium-PR-K8s-Upstream/3255/execution/node/41/log/

12:11:10  ==> box: Successfully added box 'cilium/ubuntu' (v173) for 'virtualbox'!
12:11:10    % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
12:11:10                                   Dload  Upload   Total   Spent    Left  Speed
12:11:10  
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
12:11:10  curl: (22) The requested URL returned error: 404 Not Found

I'll try one more time.

@kkourt
Copy link
Contributor Author

kkourt commented Jul 22, 2021

test-upstream-k8s

@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jul 22, 2021
@kkourt
Copy link
Contributor Author

kkourt commented Jul 22, 2021

"test-upstream-k8s" was successful, but I will do one more just go gain additional confidence.

image

@kkourt
Copy link
Contributor Author

kkourt commented Jul 22, 2021

test-upstream-k8s

@kkourt kkourt removed the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jul 22, 2021
@joestringer
Copy link
Member

Magical, it just passes. I don't know why I had such trouble with that PR on my prior backporting attempts.

@joestringer joestringer merged commit bb547b4 into cilium:v1.8 Jul 22, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jul 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/backports This PR provides functionality previously merged into master. ready-to-merge This PR has passed all tests and received consensus from code owners to merge.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants