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

ci: validate pods and systemd-networkd restart for PRs #1909

Merged
merged 16 commits into from
Apr 19, 2023

Conversation

camrynl
Copy link
Contributor

@camrynl camrynl commented Apr 11, 2023

Reason for Change:

  • A new cilium load test pipeline exists to validate that pod IPs are the same across azure endpoints and cilium endpoints. Adding a case to check CNS endpoints in memory as well.
  • Calling the load test script in the PR pipeline for cilium pod subnet and cilium overlay to strengthen test stages.
  • Also added an exit case for when privileged_pod is not found because in testing if the pod didn't come up, the script would be stuck trying to grab azure_endpoints
  • Modified cilium install from helm to kubectl apply cilium ds
  • --> Applying the daemonset used by AKS so we can apply the init container that fixed systemd-networkd restart issues.

Issue Fixed:

Requirements:

Notes:
verified script manually and on both ACN PR and load test pipeline.
Load Test Pipeline run: https://msazure.visualstudio.com/One/_build/results?buildId=71640453&view=logs&j=aafe78ca-85dd-54d8-c96a-840101a5fad4

@camrynl camrynl added the ci Infra or tooling. label Apr 11, 2023
@camrynl camrynl marked this pull request as draft April 12, 2023 00:29
@camrynl camrynl marked this pull request as ready for review April 13, 2023 01:29
@camrynl camrynl requested a review from vipul-21 April 13, 2023 01:30
@camrynl camrynl enabled auto-merge (squash) April 13, 2023 01:35
@camrynl camrynl disabled auto-merge April 13, 2023 18:49
@camrynl camrynl enabled auto-merge (squash) April 13, 2023 22:10
@@ -17,6 +17,9 @@ do
echo "Node internal ip: $node_ip"
privileged_pod=$(kubectl get pods -n kube-system -l app=privileged-daemonset -o wide | grep "$node_name" | awk '{print $1}')
echo "privileged pod : $privileged_pod"
if [ "$privileged_pod" == '' ]; then
exit 1
Copy link
Contributor

Choose a reason for hiding this comment

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

Did we encounter such case during testing ?

Can we add the status of privileged pod deployment then. ( kubectl describe daemonset privileged-daemonset -n kube-system

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

@vipul-21 vipul-21 Apr 13, 2023

Choose a reason for hiding this comment

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

Sorry I meant if we fail to get the privileged pod then it will be good to have the status of daemon set. Basically before exiting we can have that as I see from the pipeline run, daemonset.apps/privileged-daemonset created.

Ideally we should be waiting for the deployment to be complete before proceeding i think.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah makes sense, I made the switch

@camrynl camrynl requested a review from vipul-21 April 13, 2023 22:52
vipul-21
vipul-21 previously approved these changes Apr 14, 2023
@camrynl camrynl disabled auto-merge April 14, 2023 23:39
@camrynl camrynl requested a review from vipul-21 April 18, 2023 16:17
Comment on lines 52 to 59
kubectl apply -f test/integration/manifests/cilium/cilium-agent/daemonset.yaml
kubectl apply -f test/integration/manifests/cilium/cilium-agent/clusterrole.yaml
kubectl apply -f test/integration/manifests/cilium/cilium-agent/clusterrolebinding.yaml
kubectl apply -f test/integration/manifests/cilium/cilium-agent/serviceaccount.yaml
kubectl apply -f test/integration/manifests/cilium/cilium-operator/deployment.yaml
kubectl apply -f test/integration/manifests/cilium/cilium-operator/serviceaccount.yaml
kubectl apply -f test/integration/manifests/cilium/cilium-operator/clusterrole.yaml
kubectl apply -f test/integration/manifests/cilium/cilium-operator/clusterrolebinding.yaml
Copy link
Contributor

@rbtr rbtr Apr 18, 2023

Choose a reason for hiding this comment

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

kubectl apply works on directories. if these have ordering constraints, name the files with a priority prefix.

Suggested change
kubectl apply -f test/integration/manifests/cilium/cilium-agent/daemonset.yaml
kubectl apply -f test/integration/manifests/cilium/cilium-agent/clusterrole.yaml
kubectl apply -f test/integration/manifests/cilium/cilium-agent/clusterrolebinding.yaml
kubectl apply -f test/integration/manifests/cilium/cilium-agent/serviceaccount.yaml
kubectl apply -f test/integration/manifests/cilium/cilium-operator/deployment.yaml
kubectl apply -f test/integration/manifests/cilium/cilium-operator/serviceaccount.yaml
kubectl apply -f test/integration/manifests/cilium/cilium-operator/clusterrole.yaml
kubectl apply -f test/integration/manifests/cilium/cilium-operator/clusterrolebinding.yaml
kubectl apply -f test/integration/manifests/cilium/cilium-agent
kubectl apply -f test/integration/manifests/cilium/cilium-operator

Comment on lines 85 to 93
creationTimestamp: "2023-04-17T23:00:12Z"
labels:
app.kubernetes.io/managed-by: Helm
helm.toolkit.fluxcd.io/name: cilium-adapter-helmrelease
helm.toolkit.fluxcd.io/namespace: 643dcea81aff3b00014098f5
name: cilium-config
namespace: kube-system
resourceVersion: "993"
uid: 31ccec09-0511-4f18-b21a-10cf4033ce06
Copy link
Contributor

Choose a reason for hiding this comment

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

got some extra metadata here

@camrynl camrynl enabled auto-merge (squash) April 18, 2023 22:46
@camrynl camrynl requested a review from rbtr April 19, 2023 16:09
@camrynl camrynl merged commit 3f1c159 into Azure:master Apr 19, 2023
rbtr pushed a commit that referenced this pull request Sep 8, 2023
* update script to check cns in memory and add to pr pipeline

* adding stage to both overlay and podsubnet cilium stages

* add exit case if priveleged pod is not found

* check status of priv pod

* call ds status before exit

* install cilium ds with kubectl and not helm for systemd-networkd initcontainer patch

* upload cilium ds

* adding files for cilium-agent and cilium-operator deployment

* update cilium ds

* addressing comments
jpayne3506 pushed a commit that referenced this pull request Sep 11, 2023
* update script to check cns in memory and add to pr pipeline

* adding stage to both overlay and podsubnet cilium stages

* add exit case if priveleged pod is not found

* check status of priv pod

* call ds status before exit

* install cilium ds with kubectl and not helm for systemd-networkd initcontainer patch

* upload cilium ds

* adding files for cilium-agent and cilium-operator deployment

* update cilium ds

* addressing comments
@camrynl camrynl deleted the prpipeline branch September 28, 2023 16:05
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.

3 participants