feat: catalog source pod scheduling config overrides

[perdasilva]

Description of the change:
The CatalogSource CRD was recently updated to carry an additional and optional attribute grpcPodConfig as an envelope for pod spec overrides that we may want to give to the users, starting with: tolerations, nodeSelector, and priorityClassName. For instance:

apiVersion: operators.coreos.com/v1alpha1
kind: CatalogSource
...
# optional
grpcPodConfig:
  # optional
  priorityClassName: https://github.com/operator-framework/api/pull/173
  #optional
  tolerations:
    - key: some
      operator: Equal
      value: value
      effect: NoSchedule
  # optional
  nodeSelector: 
     kubernetes.io/os: linux
     some: tag

Motivation for the change:
We want to give cluster admins a way to force catalog source pods to be scheduled on specific infrastructure (i.e. nodes)

Questions

Does it make sense to expose priorityClassName and not priority?

priorityClassName seems to be used with special (custom) priority classes and system-node-critical and system-cluster-critical (which I think are reserved and very high priority). I think it should be fine to not expose the priority knob as well. I think priorityClassName covers most cases where an admin would want to affect priority. But, I'd be keen to have this assumption checked =)

Should we update the api and use *string for PriorityClassName

Since we want to override settings, it could make sense to differentiate between not set and empty. A priorityClassName of "" says to use the default priority. We don't set our own priorityClassName in our catalog source pod definition. So, the only issue here is that in case we decide to do that (which I don't think is likely), the user will not be able to unset it, i.e. override it to be "". But, even though unlikely, if we have the chance and it is quick, why not "do it right"? (if you guys also agree that seems the most correctest)

NOTE: This PR will eventually also introduce vendor changes. For reviewer sanity, I'll try to keep those off the PR for as long as possible

Reviewer Checklist

• Implementation matches the proposed design, or proposal is updated to match implementation
• Sufficient unit test coverage
• Sufficient end-to-end test coverage
• Docs updated or added to /doc
• Commit messages sensible and descriptive

[perdasilva]

/hold

[perdasilva]

holding until api changes are finalized and merged

[tylerslaton]

Looks good to me. We can probably remove the hold to see if the tests work now as it has been a number of hours since the CRD change went in.

non-block question: Do we need to write an E2E test that validates that the changes to the pod spec are actually being utilized as expected? Or is that something that is owned by some other k8s API?

[tylerslaton]

/lgtm

[perdasilva]

non-block question: Do we need to write an E2E test that validates that the changes to the pod spec are actually being utilized as expected? Or is that something that is owned by some other k8s API?

Yeah, I'll def add some e2e tests!

[njhale]

NOTE: This PR will eventually also introduce vendor changes. For reviewer sanity, I'll try to keep those off the PR for as long as possible

I think it's sufficiently clean to include vendor changes in a separate commit, that way we can still view the meaningful diffs separately and don't need to re-lgtm later.

Other than that, this is looking good! I think we just need an e2e test to make sure OLM respects these settings and updates existing catalog pods when changes are detected.

[gallettilance]

lgtm

[gallettilance]

/lgtm

[perdasilva]

non-block question: Do we need to write an E2E test that validates that the changes to the pod spec are actually being utilized as expected? Or is that something that is owned by some other k8s API?

@tylerslaton I thought about it, but yeah. That's the conclusion I reached. As long as its setting the right things in the right places the rest is up to kube

[anik120]

Looks good mostly @perdasilva , had a couple of non-blocking questions otherwise this looks good to go.

[perdasilva]

/unhold

[anik120]

/lgtm

[kevinrizza]

/approve

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: gallettilance, kevinrizza, perdasilva

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [kevinrizza]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment