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

Fix GKE Helm options for CI and docs. #12087

Merged
merged 9 commits into from Jun 18, 2020
Merged

Conversation

jrajahalme
Copy link
Member

@jrajahalme jrajahalme commented Jun 15, 2020

GKE install guide specifies the "ipam.config=kubernetes" option while
CI gkeHelmOverrides do not. Adding this option to CI gkeHelmOverrides
allowed Ginkgo runs in GKE to succeed.

Helm name for the native routing CIDR is
"global.nativeRoutingCIDR". Cilium agent command line option name is
"native-routing-cidr".

Fixes: #12053

@jrajahalme jrajahalme added area/documentation Impacts the documentation, including textual changes, sphinx, or other doc generation code. kind/bug/CI This is a bug in the testing code. release-note/misc This PR makes changes that have no direct user impact. needs-backport/1.8 labels Jun 15, 2020
@jrajahalme jrajahalme requested review from a team as code owners June 15, 2020 23:39
@maintainer-s-little-helper maintainer-s-little-helper bot added this to In progress in 1.8.0 Jun 15, 2020
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from master in 1.8.0 Jun 15, 2020
@jrajahalme
Copy link
Member Author

test-gke

@jrajahalme
Copy link
Member Author

test-me-please

@@ -92,7 +92,7 @@ below. This will ensure all pods are managed by Cilium.
--set global.cni.binPath=/home/kubernetes/bin \\
--set global.gke.enabled=true \\
--set config.ipam=kubernetes \\
--set global.native-routing-cidr=$NATIVE_CIDR
--set global.nativeRoutingCIDR=$NATIVE_CIDR
Copy link
Member

Choose a reason for hiding this comment

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

Note that we need this particular change on v1.7 as well (but not the ipam option). Given you're on backports this week, probably easiest for you to make sure the right commit from this PR goes back.

Copy link
Member

Choose a reason for hiding this comment

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

Actually, it looks like we backported this change to the docs but never backported the helm side interpretation so it has been a no-op the entire time 🤦

@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from master in 1.7.6 Jun 15, 2020
@coveralls
Copy link

coveralls commented Jun 16, 2020

Coverage Status

Coverage decreased (-0.05%) to 37.1% when pulling af11191 on pr/jrajahalme/gke-fixes into a1cc34d on master.

@jrajahalme
Copy link
Member Author

test-gke

@jrajahalme
Copy link
Member Author

Pushed one more commit only touching the GKE jenkinsfile after all other CI jobs already succeeded.

@jrajahalme
Copy link
Member Author

test-gke

1 similar comment
@jrajahalme
Copy link
Member Author

test-gke

@b3a-dev b3a-dev mentioned this pull request Jun 16, 2020
20 tasks
@jrajahalme
Copy link
Member Author

@nebril added one more commit to pass Hubble-relay image with the proper tag to Ginkgo, please have a look!

@jrajahalme
Copy link
Member Author

test-gke

@jrajahalme
Copy link
Member Author

This needs #12076, waiting it to get merged before rebasing.

@jrajahalme
Copy link
Member Author

rebased, retesting..

@jrajahalme
Copy link
Member Author

test-gke

@jrajahalme
Copy link
Member Author

@jrajahalme
Copy link
Member Author

test-gke

…docs

Basic regular expressions in sed do not support '+', while the command
line arguments for extended regular expressions are different for GNU
sed and Mac OSX. Use (unquoted) '*' instead, as there is no harm
replacing '[]' with '[]'. This form work both on Linux and Mac OSX.

Add a note to the e2e testing docs so that users know to run the
script if namespaces get stuck at 'Terminating'.

Signed-off-by: Jarno Rajahalme <jarno@covalent.io>
GKE can time out on apply when installing Cilium, use a longer timeout.

Signed-off-by: Jarno Rajahalme <jarno@covalent.io>
Create the cluster lock in namespace 'cilium-ci-lock' instead of
'default' so that it survives the CI deleting all resources in the
default namespace:

15:57:58  22:57:57 STEP: Deleting rs [lock-6f4fb6cdfb] in namespace default
15:57:58  22:57:57 STEP: Waiting for 1 deletes to return (lock-6f4fb6cdfb)
15:57:58  22:57:57 STEP: Deleting deployment [lock] in namespace default
15:57:58  22:57:57 STEP: Waiting for 1 deletes to return (lock)

Signed-off-by: Jarno Rajahalme <jarno@covalent.io>
@jrajahalme
Copy link
Member Author

test-me-please

@jrajahalme
Copy link
Member Author

test-gke

@jrajahalme
Copy link
Member Author

rebased, no changes

@jrajahalme
Copy link
Member Author

test-gke

1 similar comment
@jrajahalme
Copy link
Member Author

test-gke

@jrajahalme
Copy link
Member Author

The only change here that can affect non-GKE tests is the doubling of the Apply timeout from 30 seconds to 60 seconds. Due to the cluster locking bug all GKE test runs can step on each other at any time until this PR is merged and all PRs are rebased to include the locking fix.

There has been many successful GKE CI runs, but AFAIK all of them have been on cluster cilium-ci-13. The only other cluster I've seen selected (cilium-ci-12) has always failed tests.

Given the above this PR is ready merge.

@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 Jun 18, 2020
@joestringer joestringer merged commit 16907c4 into master Jun 18, 2020
1.8.0 automation moved this from In progress to Merged Jun 18, 2020
@joestringer joestringer deleted the pr/jrajahalme/gke-fixes branch June 18, 2020 21:14
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from master to Backport pending to v1.8 in 1.8.0 Jun 19, 2020
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from master to Backport pending to v1.8 in 1.8.0 Jun 19, 2020
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Backport pending to v1.8 to Backport done to v1.8 in 1.8.0 Jun 19, 2020
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from master to Backport pending to v1.7 in 1.7.6 Jun 30, 2020
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Backport pending to v1.7 to Backport done to v1.7 in 1.7.6 Jun 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/documentation Impacts the documentation, including textual changes, sphinx, or other doc generation code. kind/bug/CI This is a bug in the testing code. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact.
Projects
No open projects
1.7.6
Backport done to v1.7
1.8.0
  
Merged
1.8.0
Backport done to v1.8
Development

Successfully merging this pull request may close these issues.

CI/GKE: cilium pre-flight checks failed
8 participants