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

Added a cronjob to delete orphaned jenkins job namespaces #1296

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

anshulvermapatel
Copy link
Contributor

What this PR does

This PR added few more resource including a cronjob to the integration environment in the svc cluster. The cronjob is run once a day to clear the orphaned namespaces which are created by the pr_check jenkins job. Sometime, the namespaces does not get deleted due to some jenkins issue like a crashed jenkins job before executing the namespace delete logic. The cronjob targets such namespaces and delete them.

The logic is delete the namespaces which has the label sandbox-jenkins-type: aro-hcp and is >60 mins old.

Jira: ARO-10434

- name: CLIENT_ID
description: The Azure Client ID used for federation
required: true
- name: MGMT_ORPHANED_NAMESPACE
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- name: MGMT_ORPHANED_NAMESPACE
- name: ORPHANED_NAMESPACE_CLEANER

Not to confuse it with the MGMT cluster.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

- name: MGMT_ORPHANED_NAMESPACE
description: The namespace to create to have a cronjob which will delete the orphaned namespace which are not deleted due to any issues with the jenkins job.
required: true
value: orphaned-namepsace-mgmt
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
value: orphaned-namepsace-mgmt
value: orphaned-namepsace-cleaner

Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to define this namespace in the Kubernetes resources?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No actually, I intend it to be used with the default value. Let me make the required flag to false.

- apiVersion: v1
kind: ServiceAccount
metadata:
name: orphaned-namepsace-mgmt-cronjob
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we use the existing service account cluster-service-mgmt for cleanup? if we are binding the same role namespace-admin to both.
What is the advantage of defining a new service account?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since they are in different namespaces I had to create a new SA .. if we would be using the same ns where the sa cluster-service-mgmt, then we would not have created another one.

- name: KUBECTL-IMAGE
description: An image which have the `kubectl` binary in it.
required: true
value: quay.io/rhn_support_ansverma/ubi8-minimal-kubectl:latest
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this image always going to be public and available to the cronjob? If the cronjob runs on the AKS cluster, can it access Quay? IIRC there is some mechanism to pull images on demand to ACR, but maybe that's only needed for private repos.
@geoberle do you know what are the alternatives for a public image with kubectl?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This image is public, we can have this image in ACR also, however, AKS will be able to pull it. I have tested this with my mgmt AKS cluster. It worked fine.

spec:
template:
spec:
serviceAccountName: orphaned-namepsace-mgmt-cronjob
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
serviceAccountName: orphaned-namepsace-mgmt-cronjob
serviceAccountName: orphaned-namespace-mgmt-cronjob

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed it to orphaned-namespace-cleaner-cronjob

kubectl config set-cluster $CLUSTER_NAME \
--server=$KUBE_API_SERVER_HOST \
--embed-certs \
--certificate-authority=/var/run/secrets/kubernetes.io/serviceaccount/ca.crt \
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
--certificate-authority=/var/run/secrets/kubernetes.io/serviceaccount/ca.crt \
--certificate-authority=/var/run/secrets/kubernetes.io/serviceaccount/ca.crt

Is the \ at the end needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nope, will remove it.

--embed-certs \
--certificate-authority=/var/run/secrets/kubernetes.io/serviceaccount/ca.crt \

echo "setting kubectl scredential"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
echo "setting kubectl scredential"
echo "setting kubectl credential"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

kubectl config use-context current-context

echo "deleting the orphaned namespaces"
kubectl get namespaces -o json | jq -r '.items[] | select(.metadata.labels."sandbox-jenkins-type"=="aro-hcp") | select((now - (.metadata.creationTimestamp | fromdate)) / 60 > 60) | .metadata.name' | xargs kubectl delete namespace
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to handle a case where no spaces are matched?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, what do you mean by no spaces ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, a typo, a scenario where no namespaces match the filtering.

description: The namespace to create to have a cronjob which will delete the orphaned namespace which are not deleted due to any issues with the jenkins job.
required: true
value: orphaned-namepsace-mgmt
- name: KUBECTL-IMAGE
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: lowercase for consistency with the other env vars

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay.

Comment on lines 124 to 133
kubectl config set-cluster $CLUSTER_NAME \
--server=$KUBE_API_SERVER_HOST \
--embed-certs \
--certificate-authority=/var/run/secrets/kubernetes.io/serviceaccount/ca.crt \

echo "setting kubectl scredential"
kubectl config set-credentials $SERVICE_ACCOUNT_NAME \
--token=$(cat /var/run/secrets/kubernetes.io/serviceaccount/token)
kubectl config set-context current-context --cluster=$CLUSTER_NAME --user=$SERVICE_ACCOUNT_NAME
kubectl config use-context current-context
Copy link
Collaborator

Choose a reason for hiding this comment

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

since the pod runs within the cluster, there is no need to set up a kubeconfig. just make sure that the job runs as the service account specified by $SERVICE_ACCOUNT_NAME

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ohh yeah, you are right. I did the changes, testing them, once done, I will push the changes.

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.

3 participants