Insights Operator pulling and exposing data from the OCM API

[tremes]

No description provided.

[openshift-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please assign staebler after the PR has been reviewed.
You can assign the PR to them by writing /assign @staebler in a comment when ready.

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

Needs approval from an approver in each of these files:

• OWNERS

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

[sbose78]

Does insights Operator have access to the org ID and/or cluster Id?

[tremes]

@sbose78 The Insights Operator does have access to the cluster ID, but from what I know it doesn't have access to the org ID.

[iNecas]

/lgtm
/approve

[sbose78]

Thanks Ivan, I think we have all the approvals needed at this stage.

[sbose78]

Hi @smarterclayton Would you be able to review this, and potentially merge this ?

[tremes]

@sbose78 Thanks for pushing it, but maybe it would be good if someone from the OCM can review too. @abhgupta pls

[petli-openshift]

LGTM from my perspective.

[sbose78]

I'd like to address a couple of questions I see in the above in multiple discussion threads.

How does an entity make the SCA cert accessible to service accounts in other namespaces ?

• Make the Secret etc-pki-entitlement available for use in other namespaces by creating a cluster-scoped Share resource

apiVersion: projectedresource.storage.openshift.io/v1alpha1
kind: Share
metadata:
  name: etc-pki-entitlement
spec:
  backingResource:
    kind: Secret
    apiVersion: v1
    name: etc-pki-entitlement
    namespace: openshift-config-managed

• Admin creates a clusterrolebinding to allow a service account ( example, pod-sa ) access to the above Share resource ( sample )

Consumption by Pods

kind: Pod
apiVersion: v1
metadata:
  name: user-workload
  namespace: foo
spec:
  serviceAccountName: pod-sa  # <----- Previously a clusterrolebinding was created for this sa
  containers:
    - name: my-frontend
      image: quay.io/quay/busybox
      volumeMounts:
        - mountPath: "/data"
          name: my-csi-volume
      command:
        - sh
        - -c
        - |
          while true
          do
            ls -la /data
            touch /data/foo
            ls -la /data
            echo "calling sleep"
            sleep 120
          done
  volumes:
    - name: my-csi-volume
      csi:
        #readOnly: true
        driver: csi.shared-resources.openshift.io
        volumeAttributes:
          share: etc-pki-entitlement. # <----- name of the cluster-scoped `Share` resource.

Consumption by Builds

After https://issues.redhat.com/browse/BUILD-274 is done, one would be able to do the following:

kind: Build
apiVersion: build.openshift.io/v1
metadata:
  ...
spec:
  source:
     ...
  strategy:
    dockerStrategy:
      volumes:
      - name: etc-pki-entitlement
        type: CSI
        csi: 
          driver: projected-resource.csi.openshift.io
          volumeAttributes:
            share: etc-pki-entitlement
      volumeMounts:
      - name: etc-pki-entitlement
        mountPath: /etc/pki/entitlement

As of today, the Build API team has made progress with supporting Secret & ConfigMap volume mounts in Builds as part of https://issues.redhat.com/browse/BUILD-87 . https://issues.redhat.com/browse/BUILD-274 is a candidate for 4.10 .

[sbose78]

As Ben mentioned, this EP's sole goal is to ensure the SCA certs are pulled into the cluster and lifecycle'd by the Insights Operator.

Consumption of the same is being driven by work in distinct Openshift components:

• The brand new OpenShift Projected Resource CSI Driver
• Changes in the Build API to support volume mounts ( specifically, CSI volume mounts )

[tremes]

I slightly updated the proposal to address some points and cover the part of SCA certs use. Can you @bparees, @dhellmann please take a look at my last commit? Let's discuss potential blockers tomorrow.

[sbose78]

Requesting one change : Create the Share resource.

[wking]

Since most consumers will presumably be mounting the Share, maybe this integration test should use that approach instead of shortcutting to use the Secret directly?

[bparees]

this was discussed in a since-resolved comment thread somewhere, but.... i don't want the insights operator team's testing to be dependent on Share behavior.

they have the ability to test their functionality end to end directly, so they should do that.

The team that owns the Shared-Resource driver+builds should have tests that ensure they can consume this content successfully at their end.

[adambkaplan]

I think the bits involving the Share resource should ultimately live in the OpenShift build suite. Test plan as follows:

1. insights operator obtains SCA cert from OCM
2. insights operator creates Share resource and ClusterRoleBinding
3. ocp build suite creates a build that does a yum install of subscription-only content

[wking]

Not covered in this sentence is the structure of the Secret. Will a particular well-known key be used? If so, can we document it here, or is that a low-enough level of detail that it's not worth including in the enhancement proposal?

[adambkaplan]

IIRC the key name can be any .pem file. On a RHEL system the subscription cert key names have what appear to be a uuid for the file name.

[tremes]

Maybe we should mention it here. I am using kubernetes.io/tls secret with tls.crt for the certificate and tls.key for the private key.

[wking]

If a cluster is downgraded to a version that does not poll for entitlement updates, will that version of the insights operator (4.8.z?) have logic around to remove the etc-pki-entitlement secret and other cruft to keep in-cluster components from trying to consume stale data? If the insights operator is downgraded before some consumer (builds and/or CSI drivers?), will the higher-version consumers gracefully handle the consumed secret's removal?

[bparees]

If a cluster is downgraded to a version that does not poll for entitlement updates, will that version of the insights operator (4.8.z?)

for tech preview it's not applicable since upgrade/downgrade isn't allowed, but in general if you downgrade below the level at which the insights operator has this behavior then yes, i'd expect the content to become stale (i'm not sure how long the tokens are good for)

i'm not sure how you'd propose to solve that on downgrade, though. the older version of the operator wouldn't even know about the content to remove it(even if we could agree that was the right thing to do, which i don't think i do)

[tremes]

Yes this sounds like an edge case to me and yes there will be a stale secret in such case.

[adambkaplan]

Degraded means the cluster can't upgrade. Are we sure we want this?

[bparees]

yes, we want it.

1. this is tech preview. we should absolutely gather data on whether this operator is ending up degraded during the tech preview period before making a final decision for GA

2. degraded doesn't block z-stream upgrades, and can be overridden for y-stream upgrades, the point is it's a conscious choice. I gave this (somewhat dubious) example in another comment thread on this EP: suppose you have pods that need this entitlement to do work at startup. Doing an upgrade is going to trigger all those pods to restart. You want to know, before you upgrade your cluster, that you don't have valid entitlements on your cluster, or all those pods are going to fail to restart after the node reboots triggered by the upgrade.

fundamentally this operator has a job to do, and if it's not doing the job, it should report degraded(after a reasonable period of time)

[bparees]

that said, going degraded should only happen after some number of retries or period of time. not on first failure.

[bparees]

and needs to take into account disconnected clusters (i.e. it shouldn't be degraded on a cluster w/ no network access)

[adambkaplan]

IIRC the key name can be any .pem file. On a RHEL system the subscription cert key names have what appear to be a uuid for the file name.

[adambkaplan]

I think the bits involving the Share resource should ultimately live in the OpenShift build suite. Test plan as follows:

1. insights operator obtains SCA cert from OCM
2. insights operator creates Share resource and ClusterRoleBinding
3. ocp build suite creates a build that does a yum install of subscription-only content

[adambkaplan]

Note that the Share bits are now planned as tech preview for OCP 4.10

[tremes]

Does it mean that it will block the graduation criteria from the TP to GA mentioned here? If so then we would need to go GA with your bits...I guess

[adambkaplan]

Ditto my comments above - degraded means no cluster upgrades. Do we want this?

[adambkaplan]

It appears that prior GH comments we reached consensus that Degraded is a desirable condition. I would like to see this justification reflected in the proposal itself.

[bparees]

this was discussed in a since-resolved comment thread somewhere, but.... i don't want the insights operator team's testing to be dependent on Share behavior.

they have the ability to test their functionality end to end directly, so they should do that.

The team that owns the Shared-Resource driver+builds should have tests that ensure they can consume this content successfully at their end.

[bparees]

If a cluster is downgraded to a version that does not poll for entitlement updates, will that version of the insights operator (4.8.z?)

for tech preview it's not applicable since upgrade/downgrade isn't allowed, but in general if you downgrade below the level at which the insights operator has this behavior then yes, i'd expect the content to become stale (i'm not sure how long the tokens are good for)

i'm not sure how you'd propose to solve that on downgrade, though. the older version of the operator wouldn't even know about the content to remove it(even if we could agree that was the right thing to do, which i don't think i do)

[bparees]

yes, we want it.

1. this is tech preview. we should absolutely gather data on whether this operator is ending up degraded during the tech preview period before making a final decision for GA

2. degraded doesn't block z-stream upgrades, and can be overridden for y-stream upgrades, the point is it's a conscious choice. I gave this (somewhat dubious) example in another comment thread on this EP: suppose you have pods that need this entitlement to do work at startup. Doing an upgrade is going to trigger all those pods to restart. You want to know, before you upgrade your cluster, that you don't have valid entitlements on your cluster, or all those pods are going to fail to restart after the node reboots triggered by the upgrade.

fundamentally this operator has a job to do, and if it's not doing the job, it should report degraded(after a reasonable period of time)

[bparees]

that said, going degraded should only happen after some number of retries or period of time. not on first failure.

[bparees]

and needs to take into account disconnected clusters (i.e. it shouldn't be degraded on a cluster w/ no network access)

[bparees]

/approve

looks like there might be a few final items @adambkaplan would like to see explicitly stated in the EP, but in general i think the critical concerns are addressed and/or roadmapped.

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bparees, iNecas

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 [bparees]

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

[tremes]

Thanks @bparees. I added one more commit mentioning the reasons for the decision on the degraded status.

[bparees]

/lgtm

[sbose78]

FYI, the consumption APIs are undergoing changes
openshift/csi-driver-shared-resource#55