Skip to content

Conversation

@camrynl
Copy link
Contributor

@camrynl camrynl commented Jul 21, 2023

Reason for Change:

  • This PR updates cilium operator arguments for cilium identities to lower the garbage collection interval.
  • Adding a stage in the nightly pipeline to verify that cilium identities are removed once the cilium endpoint is deleted.

This check will only run for the nightly pipeline and will be skipped in the PR runs. It was previously added to the old version of our nightly pipeline.

Issue Fixed:

Requirements:

Notes:

@camrynl camrynl added the ci Infra or tooling. label Jul 21, 2023
@camrynl camrynl requested review from jpayne3506 and vipul-21 July 21, 2023 22:03
@camrynl camrynl requested a review from a team as a code owner July 21, 2023 22:03
@paulyufan2 paulyufan2 force-pushed the camrynl/ciliumnightlypipeline branch from 00fc095 to 8390b97 Compare July 24, 2023 17:05
else
echo "skip cilium identities check for PR pipeline"
fi
name: "CiliumIdentites"
Copy link
Contributor

Choose a reason for hiding this comment

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

NIT: Typo: CiliumIdentities

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks, fixed

echo "skip cilium identities check for PR pipeline"
fi
name: "CiliumIdentites"
displayName: "Verify Cilium Identity Deletion"
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: should we use plural? displayName: "Verify Cilium Identities Deletion"

Comment on lines 178 to 192
while true; do
pods=$(kubectl get pods -n $ns --no-headers=true 2>/dev/null)
if [[ -z "$pods" ]]; then
echo "No pods found"
break
fi
sleep 2s
done
sleep 20s
echo "Verify cilium identities are deleted"
checkIdentity="$(kubectl get ciliumidentity -o json | grep cilium-test | jq -e 'length == 0')"
if [[ -n $checkIdentity ]]; then
echo "##[error]Cilium Identities still present"
else
printf -- "Identities deleted\n"
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we have additional logging here? If identities are still present -> see what they are and confirm which identities are being deleted (if possible).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, the check is verifying that the identities are being deleted from the cilium-test ns. I updated the logging with references to the namespace.

fi
sleep 2s
done
sleep 20s
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are sleeping again after checking the deletion of pods ? For cilium gc ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, need to wait for cilium gc

if [[ -n $checkIdentity ]]; then
echo "##[error]Cilium Identities still present in cilium-test namespace"
else
printf -- "Identities deleted from cilium-test namespace\n"
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason we added printf and not echo

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When I wrote this for the original nightly pipeline I recall I had to change the first case to echo with azp syntax in order to indicate actual failure in the pipeline.

@camrynl camrynl force-pushed the camrynl/ciliumnightlypipeline branch from 3bb6c79 to df2d839 Compare July 25, 2023 17:42
@camrynl camrynl enabled auto-merge (squash) July 25, 2023 17:43
@camrynl camrynl merged commit ca02c14 into master Jul 25, 2023
@camrynl camrynl deleted the camrynl/ciliumnightlypipeline branch July 25, 2023 19:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci Infra or tooling.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants