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 ARC Conformance Script #1585

Merged
merged 1 commit into from
Oct 22, 2024
Merged

Fix ARC Conformance Script #1585

merged 1 commit into from
Oct 22, 2024

Conversation

mateoflorido
Copy link
Member

Overview

This PR fixes the commands used in the ARC conformance script and includes some minor improvements to capture better insights in case of failures.

Rationale

The following error was caught during a conformance test run:

sed: -e expression #2, char 141: unknown option to `s'

In this PR, the script now uses | as the delimiter in sed commands instead of / to avoid issues with escaping paths. Additionally, echo has been replaced with printf to ensure the SAS Token is treated as a literal string. Finally, we extract the logs from the completed container in case it's not running, helping us understand what's failing.


Mark below if this PR requires a job refresh (jjb) after merge:

  • Needs jjb after merge

Please make sure to open PR's against the correct code:

  • For integration tests, please make changes in jobs/integration
  • For validation envs, jobs/validate
  • For MicroK8s,jobs/microk8s
  • For charm/bundle builds, jobs/build-charms

done
else
echo "No ARC pod running to copy results from"
kubectl logs -n ${ARC_NS} ${ARC_POD} > arc-logs
Copy link
Member

Choose a reason for hiding this comment

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

how do we cat logs from a pod that's not running? Is this because the pod WAS running at some point and these are the left over logs from it

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. In this case, the pod should be in a completed state (due to a failure in the conformance run), and you should be able to pull the logs from the pod. For example:

root@t-1:~/azure-arc-validation/testsuite# k8s kubectl logs -n $ns azure-arc-kubernetes-conformance-7zx9g
Error: Please set global.SUBSCRIPTION_ID in property file
Error: Please set global.CLIENT_ID in property file
Error: Please set global.TENANT_ID in property file
root@t-1:~/azure-arc-validation/testsuite# k8s kubectl get po -A
NAMESPACE                          NAME                                     READY   STATUS      RESTARTS   AGE
azure-arc-kubernetes-conformance   azure-arc-kubernetes-conformance-7zx9g   0/1     Completed   0          57s
azure-arc-release                  cluster-diagnostic-checks-job-lz9pk      0/1     Completed   0          120m
kube-system                        cilium-operator-7948fd8b8-pwtgv          1/1     Running     0          3h10m
kube-system                        cilium-t8xmb                             1/1     Running     0          3h10m
kube-system                        ck-storage-rawfile-csi-controller-0      2/2     Running     0          3h10m
kube-system                        ck-storage-rawfile-csi-node-4q2rw        4/4     Running     0          3h10m
kube-system                        coredns-5858b47b85-mwcpw                 1/1     Running     0          3h10m
kube-system                        metrics-server-84648cc687-rc4wk          1/1     Running     0          3h10m

Copy link
Member

@addyess addyess left a comment

Choose a reason for hiding this comment

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

I trust your next-level bash scripting here.

@mateoflorido mateoflorido merged commit 61176be into main Oct 22, 2024
6 checks passed
@mateoflorido mateoflorido deleted the KU-1779/arc-conformance branch October 22, 2024 18:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants