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

.github/workflows: wait for install job before checking aws-node DaemonSet #439

Closed
wants to merge 1 commit into from

Conversation

tklauser
Copy link
Member

@tklauser tklauser commented Jul 16, 2021

Wait until the cilium-cli-install job completed and Cilium is installed
before checking the status of the aws-node DaemonSet. Otherwise, we
might fail the check if the aws-node DaemonSet is still terminating.

@tklauser tklauser temporarily deployed to ci July 16, 2021 13:36 Inactive
@tklauser tklauser force-pushed the pr/tklauser/aws-node-wait branch from f004c46 to d1d73ab Compare July 16, 2021 15:31
@tklauser tklauser temporarily deployed to ci July 16, 2021 15:31 Inactive
@tklauser tklauser force-pushed the pr/tklauser/aws-node-wait branch from d1d73ab to 66d649f Compare July 19, 2021 07:10
@tklauser tklauser temporarily deployed to ci July 19, 2021 07:10 Inactive
@tklauser tklauser changed the title .github/workflows: wait for aws-node pods to be deleted .github/workflows: wait for install job before checking aws-node DaemonSet Jul 19, 2021
@tklauser tklauser force-pushed the pr/tklauser/aws-node-wait branch from 66d649f to fc0fd7c Compare July 19, 2021 07:36
@tklauser tklauser temporarily deployed to ci July 19, 2021 07:36 Inactive
@tklauser tklauser force-pushed the pr/tklauser/aws-node-wait branch from fc0fd7c to 73c2be9 Compare July 19, 2021 08:11
@tklauser tklauser temporarily deployed to ci July 19, 2021 08:11 Inactive
…onSet

Wait until the cilium-cli-install job completed and Cilium is installed
before checking the status of the aws-node DaemonSet. Otherwise, we
might fail the check if the aws-node DaemonSet is still terminating.

Signed-off-by: Tobias Klauser <[email protected]>
@tklauser
Copy link
Member Author

Both EKS workflows passed:

Removing test commit and marking as ready to review.

@tklauser tklauser marked this pull request as ready for review July 19, 2021 08:46
@tklauser tklauser requested review from a team as code owners July 19, 2021 08:46
@tklauser tklauser requested a review from nebril July 19, 2021 08:46
@tklauser tklauser requested a review from joestringer July 19, 2021 08:46
@tklauser tklauser force-pushed the pr/tklauser/aws-node-wait branch from 73c2be9 to a7b3e89 Compare July 19, 2021 08:46
@tklauser tklauser temporarily deployed to ci July 19, 2021 08:46 Inactive
@nbusseneau nbusseneau self-requested a review July 19, 2021 09:23
@@ -6,5 +6,4 @@ set -e
# Install Cilium
cilium install \
--cluster-name "${CLUSTER_NAME}" \
--wait=false \
Copy link
Member

Choose a reason for hiding this comment

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

We already actively wait immediately after enabling Hubble in the next script. Initially, the whole point of splitting the EKS scripts into 2 separate install/test steps was to take advantage of the fact we install into clusters without any nodes, with nodes being added later.

Now that we don't have that and instead create clusters with nodes available right away, I think the split is not warranted anymore and we should switch back to a single script that works like the others.

Copy link
Member

@nbusseneau nbusseneau Jul 19, 2021

Choose a reason for hiding this comment

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

(following offline discussion: proposal in #442)

@tklauser
Copy link
Member Author

Let's close this in favor of #442.

@tklauser tklauser closed this Jul 19, 2021
@tklauser tklauser deleted the pr/tklauser/aws-node-wait branch July 19, 2021 10:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants