Use healthcheck cron job to determine cluster readiness #743
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,71 @@ | ||
| package healthchecks | ||
|
|
||
| import ( | ||
| "context" | ||
| "fmt" | ||
| "log" | ||
|
|
||
| "github.com/openshift/osde2e/pkg/common/logging" | ||
| batchv1 "k8s.io/api/batch/v1" | ||
| metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||
| "k8s.io/apimachinery/pkg/watch" | ||
| "k8s.io/client-go/kubernetes" | ||
| ) | ||
|
|
||
| // CheckHealthcheckJob uses the `osd-cluster-ready` healthcheck job to determine cluster readiness. If the cluster | ||
| // is not ready, it will return an error. | ||
| func CheckHealthcheckJob(k8sClient *kubernetes.Clientset, ctx context.Context, logger *log.Logger) error { | ||
| logger = logging.CreateNewStdLoggerOrUseExistingLogger(logger) | ||
|
|
||
| logger.Print("Checking whether cluster is healthy before proceeding...") | ||
|
|
||
| bv1C := k8sClient.BatchV1() | ||
| namespace := "openshift-monitoring" | ||
| name := "osd-cluster-ready" | ||
| jobs, err := bv1C.Jobs(namespace).List(ctx, metav1.ListOptions{}) | ||
| if err != nil { | ||
| return fmt.Errorf("failed listing jobs: %w", err) | ||
| } | ||
| for _, job := range jobs.Items { | ||
| if job.Name != name { | ||
| continue | ||
| } | ||
| if job.Status.Succeeded > 0 { | ||
| log.Println("Healthcheck job has already succeeded") | ||
| return nil | ||
| } | ||
| log.Println("Healthcheck job has not yet succeeded, watching...") | ||
| } | ||
| watcher, err := bv1C.Jobs(namespace).Watch(ctx, metav1.ListOptions{ | ||
| ResourceVersion: jobs.ResourceVersion, | ||
| FieldSelector: "metadata.name=osd-cluster-ready", | ||
| }) | ||
| if err != nil { | ||
| return fmt.Errorf("failed watching job: %w", err) | ||
| } | ||
| for { | ||
| select { | ||
| case event := <-watcher.ResultChan(): | ||
| switch event.Type { | ||
| case watch.Added: | ||
| fallthrough | ||
| case watch.Modified: | ||
| job := event.Object.(*batchv1.Job) | ||
| if job.Status.Succeeded > 0 { | ||
|
There was a problem hiding this comment. I think we need to handle "completed but failed" as well. ...But how we handle it is different before vs after openshift/configure-alertmanager-operator#143
Need to think through this some more. There was a problem hiding this comment. I'll wait to hear your updated thoughts on how this should work before altering this logic. There was a problem hiding this comment. Talked this through with @jharrington22 just now. We're going to rework the readiness side so the picture is clear. Specific to this case, I think for osde2e's purposes "completed but failed" will translate to "cluster not ready, ain't ever gonna be". But I'm also pretty sure we're going to use prometheus to carry that state... which would mean substantial changes in this PR. Stay tuned -- I hope to have something written up by tomorrow. There was a problem hiding this comment. Keep an eye on OSD-6646 starting here There was a problem hiding this comment. "Completed but failed" should be treated as "cluster not ready, ain't ever gonna be". I'll post more info in a top-level comment. |
||
| return nil | ||
| } | ||
| if job.Status.Failed > 0 { | ||
| return fmt.Errorf("cluster readiness job failed") | ||
| } | ||
| case watch.Deleted: | ||
| return fmt.Errorf("cluster readiness job deleted before becoming ready (this should never happen)") | ||
| case watch.Error: | ||
| return fmt.Errorf("watch returned error event: %v", event) | ||
| default: | ||
| logger.Printf("Unrecognized event type while watching for healthcheck job updates: %v", event.Type) | ||
| } | ||
| case <-ctx.Done(): | ||
| return fmt.Errorf("healtcheck watch context cancelled while still waiting for success") | ||
| } | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not super familiar with client-go, but why doesn't
bv1C.Jobs(namespace).Get(ctx, name, metav1.GetOptions{})work here?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sigh... It does. I just did it a dumb way because I too am not very familiar with client-go.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wait! On second thought, it does need to be a list. I need the resourceVersion field of the list itself in order to initiate the watch later. This allows me to detect the creation and deletion of jobs matching my criteria since I performed the list.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I understand.
If you use a zero resourceVersion, do you even need the initial get/list?