redhat-dotnet-imagestreams: add .NET 9.0 images, remove EOL .NET 7 images.

[tmds]

No description provided.

[tmds]

@omajid @aslicerh @komish ptal.

Besides adding the 9.0 version and removing the 7.0 version, I've also refactored the chart so it uses templates to avoid the duplication. For future version updates we can then limit ourselves to changing the Chart.yaml and data.yaml files.

The goal is to merge this at the 9.0 GA which will then make the new chart available to existing OpenShift installations.

[omajid]

Thanks, this is amazing!

[komish]

@tmds

The chart is currently failing chart testing when chart-verifier is running against it. I know we're only installing imagestreams here, but does it make sense to include a basic test that confirms the imagestream installed without issue?

[tmds]

does it make sense to include a basic test that confirms the imagestream installed without issue?

I'm trying to figure out what looks like.

I see many charts include a templates/tests directory, but it seems they aren't using the image streams in the test definition. Instead they refer directly to an registry.access.redhat.com image. For example: https://github.com/openshift-helm-charts/charts/blob/main/charts/redhat/redhat/redhat-python-imagestreams/0.0.2/src/templates/tests/test-import-imagestream.yaml#L12

Do these tests rely on the presence of any test causing the image streams to get imported before running that test, and in that manner verify the imagestreams are importable?

[komish]

I'm seeing the tags key as invalid, and should either be in an enumerated dependency (where it has some ability to impact installation behavior [1], or added as an annotation to the parent chart where it doesn't.

[tmds]

I'll remove it. Note that we included tags because the other image streams charts in the repo have it as well.

[komish]

Do these tests rely on the presence of any test causing the image streams to get imported before running that test, and in that manner verify the imagestreams are importable?

Yes. Traditional helm testing (e.g. helm test <release> will let the user install the release, then explicitly invoke its tests afterwards. Our pipeline uses chart-testing which completes both of those tasks, so it's fair to conclude that the chart will be installed and then the test will be executed. Therefore, if a test is executed, then it's reasonable to assume the helm installation phase has succeeded. Especially for these smaller charts that install things like imagestreams, which aren't technically "workloads".

I see many charts include a templates/tests directory, but it seems they aren't using the image streams in the test definition. Instead they refer directly to an registry.access.redhat.com image. For example: https://github.com/openshift-helm-charts/charts/blob/main/charts/redhat/redhat/redhat-python-imagestreams/0.0.2/src/templates/tests/test-import-imagestream.yaml#L12

To my eye, this test looks invalid. pod.spec.lookupPolicy doesn't appear to be a valid configuration. There's a good chance that imagestream chart was accepted via the "demotion" path that the pipeline offers to redhat charts, which makes helm lint the only "requirement".

I think a valid test might look something like this:

apiVersion: v1
kind: Pod
metadata:
  name: "{{ .Release.Name }}-imagestream-test"
  namespace: "{{ .Release.Namespace }}"
  annotations:
    "helm.sh/hook": test
    "alpha.image.policy.openshift.io/resolve-names": '*'
spec:
  containers:
    - name: dotnet-test
      image: dotnet:8.0-ubi8
      imagePullPolicy: IfNotPresent
      command:
        - '/bin/bash'
        - '-ec'
        - 'echo helloworld'
    - name: dotnet-runtime
      image: dotnet-runtime:8.0
      imagePullPolicy: IfNotPresent
      command:
        - '/bin/bash'
        - '-ec'
        - 'echo helloworld'
  restartPolicy: Never

Notably, the alpha.image.policy.openshift.io/resolve-names seems to be a required annotation that allows a Pod to resolve the shortname reference of the imagestream. You can probably optimize this a bit more to avoid hardcoding the image shortname references. Either way, in my use of this test, I get a successful test outcome against an arbitrary release of this chart in an arbitrary namespace.

The ability to install this chart in any namespace is critically important. I see your values.yaml includes a namespace definition, but it doesn't seem to be referenced anywhere. In our hosted pipeline, charts are installed to a fresh namespace, so if this hardcoded namespace 'openshift' were actually used, I'm sure this test would fail.

Hope this helps. Let me know how else I can assist.

[tmds]

The ability to install this chart in any namespace is critically important. I see your values.yaml includes a namespace definition, but it doesn't seem to be referenced anywhere. In our hosted pipeline, charts are installed to a fresh namespace, so if this hardcoded namespace 'openshift' were actually used, I'm sure this test would fail.

This was also copied from other imagestream charts in the repo. Probably this issue applies to all of them.

[tmds]

@komish I've added a test based on your example. I assume it is not running in CI currently? Can you trigger it to know how it behaves?

Also: what should I do about the values.yaml namespace? I went through some charts in the repo, but I haven't found the answer.

[komish]

@tmds

I've added a test based on your example. I assume it is not running in CI currently? Can you trigger it to know how it behaves?

Easiest thing to do is install chart-testing and install your chat from a local repo. Requires some setup, but will be the fastest thing.

Otherwise, just helm install and helm test. It's not purely apples-to-apples, but it's close enough.

Also: what should I do about the values.yaml namespace? I went through some charts in the repo, but I haven't found the answer.

I don't think you need this, but it's up to you and your chart's design. Ultimately, Helm provides .Release.Namespace which is most likely set by either the --namespace flag passed into helm install. Alternatively, if you don't provide one, I would presume that your kubeconfig's current-context drives what namespace it gets installed to.

All that to say - you have options here, and I'm not sure if namespace is necessary in your values.yaml. To that end, you should be okay to remove it so long as it makes sense to you.

[tmds]

Otherwise, just helm install and helm test. It's not purely apples-to-apples, but it's close enough.

This is what I ended up doing.

All that to say - you have options here, and I'm not sure if namespace is necessary in your values.yaml. To that end, you should be okay to remove it so long as it makes sense to you.

I've removed it, and I think it would make sense for the other charts to do the same. The parameter implies the install would be in that namespace, but as you have pointed out: it uses the current context instead.

The test is now checking all tags of the image streams and whether those include the right .NET versions for runtime/SDK.
It uncovered a mistake I had made for the runtime image repository names.

@komish thanks for all your help!!