Closed dee0 closed 1 week ago
@Skarlso , although only starting officially on Friday, maybe you can have a first look at this issue (#353, open-component-model/ocm-project#71, open-component-model/ocm-project#70 and open-component-model/ocm-project#69 are related to the same scenario Dan's testing)?
Hello. :)
Resource is a standalone thing. I'll have to read this again to figure out what you are looking for.
Please include the setup you are trying to attain / test in yaml format.
You don't need the Resource object for Helm Charts. You should be able to define the resource for Configuration or Locatlization. The Resource is only used if you don't want to do any of the two others ( Loc or Conf ).
Hey @Skarlso
I have attached component-and-config.zip which contains :
These should be the file versions I was using when I first ran into problems. Obviously with these files I was hoping my helm chart would be installed. However additional goals were
The attached almost worked. Using crane I could see that localization and configuration worked and a helm repo and helm release were created. However helm was unable to use the output from the localization because it requires a chart be in a namespace repo.
This work around was put in place to make helm happy by adding a namespace to the repo in the case that an extra 'helm' identify has been added to the resource. However as I mentioned in the description above the only way to trigger this work around is to use a Resource.
And as stated above, even if you do this you still don't get something that works for the other reasons I mentioned in the initial description.
Hm, can you provide something that I can use to reproduce your problem locally?
From the component it looks like your chart resource isn't a chart resource. It's just a tarred up folder. You need to define a helmChart
type resource in order for it to be handled as a helm chart. Otherwise, the created OCI repository will not have the right header values to identify it as such.
Hey @Skarlso That doesn't work because then
I think it should be pretty easy to the scenario we want to have work. This by either
Localization already has an output snapshot. 🤔 There is no point on having a Resource after localization.
I'm still not sure what you are trying to accomplish. If the resource is not marked as a helm chart it will not be handled as a helm chart. We can't detect if it's a helm chart otherwise.
The chart is in a separate repo, which is something we don't want
I'm not sure I follow this. Can you elaborate what your mean by separate repo? And you "don't want that"?
For reference, here is a working helm chart example https://github.com/Skarlso/ocm-helm
I'll analyze the rest of the issue later. :) Not sure about that latest tag thing. :D
In the attached image I have circled in red the extra repo. The problem with the extra repo is that then we have to negotiate with the teams managing the various repositories we have to work with to request permission for 2 repositories instead of 1. Basically it will double the 'paperwork' involved in getting this to work.
Hey @Skarlso Is that sample actually working? I see there is a Localization but I don't 'backend-end-config' definition anywhere.
Btw, in my case Localization and Configuration are updating the values.yaml of the helm chart. Not sure if that is clear with the data I provided so far.
As stated in the bug description, to get something to work for my PoC I followed the ocm-setup-demo. Of course that deviates from our use case quite a bit so isn't something we would actually adopt.
ocm-setup-demo was waaaay before we started supporting helm chart so that "cheats" a bit by doing some manual things with the charts and preloading them. :)
uh, it used to work. 🤔 but we changed to cue lang for values. So you're probably right that the values file things no longer works. We ran out of time to update all samples sadly.
I'll try testing helm access once I get to this issue. :) The first intended case should have worked 🤔 Not sure why latest
was used I'll have to go through the code.
Thanks for reporting it. Helm chart support is very much an alpha feature. I'm sure it has some hiccups especially since the requirement for named repositories.
Okay so, the helm chart downloading and creating a snapshot that is a proper helm chart repo with resource is working.
However...
The configuration and localization functionality for helm charts is missing because it doesn't re-package it properly and creates a new helm chart out of it after configuring or updating a values.yaml file.
This is a missing feature, basically. We didn't quite have the time to finish it.
The Resource
part is working however. To reproduce it, see an example here: https://github.com/Skarlso/ocm-helm
Which creates the following snapshot value:
Status:
Conditions:
Last Transition Time: 2024-03-26T12:01:45Z
Message: Snapshot with name 'ocm-with-helm-deployment-mw73dth' is ready
Observed Generation: 1
Reason: Succeeded
Status: True
Type: Ready
Digest: sha256:31b957024d57c2b4768c2a5ff9004fe0acdbbcf6974167fa0a95d858c21ea985
Observed Generation: 1
Repository URL: https://registry.ocm-system.svc.cluster.local:5000/sha-785370814667106374/podinfo
Tag: 6.3.5
@Skarlso and @dee0 , the current way to do configuration and localization for me feels a bit to cumbersome. Thinking of people that want to deploy their Helm Chart, but understood that it makes sense to model their product in OCM should have a very simple possibility to configure and localize their application, similar to how they would by modifying the values.yaml, to specify their intent. If in the background we require CRs with a complex structure and content that establish the intent in the system, then we at least should check if it is possible to at least offer a more simple interface at the surface. Any ideas to solve this Gordian knot are welcome :-)
That is not going to be an easy task. :/ But I think it's gonna be fine maybe. It's just the re-packaging that's missing. A user would have a separate resource that contains configuration values perhaps, and we could merge that with existing ones, I don't know.
Another idea was to basically allow people to describe how to configure something by having the WASM scripts in a resource that would do the configuration for us. They are tiny binaries after all and we could just run them as is and it would configure the requested resource.
That's also an option.
WASM was and still is an interesting idea for the configuration and also still a hot topic out there. I would even buy something super simple for the bread and butter cases and something more complex for the other cases, but I really think that adoption will not raise when we keep the interface as complex as it is.
That might be true, yeah.
I have to do some brainstorming with myself and see if I can come up with something. Neverthless, a pluggable infrastructure is still the way to go I believe. It has been wildly successful in other areas after all.
Hey @Skarlso & @morri-son
Want to restate the use case in our division and then present my naive thoughts on supporting them.
Use case :
From my naive user perspective, what is missing to support the above 'nicely' is:
And with these mods I could have the simple 'pipeline' of ComponentVersion -> Localization ( helm chart ) -> Configuration ( helm chart ) -> FluxDeployer as mentioned in the description of this ticket.
That said, even a change as simple as what is in this PR would make it so my use case is mostly supported. The only catches would be
Fwiw, I was hoping to step through the code to see how hard the naive solution above would be to implement. This is why I left this PR in the 'work in progress' state. Didn't find the time to do it today but maybe I'll get to it this week sometime.
Hello, let me try to clarify some things.
Outside of the transported component case, the number of OCI repositories required should be minimized. i.e. If I have a Component repository already and I can have my helm chart as a local resource then I should not need yet another repository just for my helm chart.
I assume you are talking about our local registry? That is not something the service or the infrastructure team would be aware off at all. That registry is ocm-controller's local cache and an integration point with flux itself. The FluxDeployer creates Flux components to be able to deploy... with Flux. :) We are aiming to create other deployers like ArgoDeployer or something that can provide other kind of options, but that's in the future. For now, all you would need to be concerned about is defining the properties for a helm chart.
Unfortunately I can't actually set the helmChart extra identity when defining my component because of https://github.com/open-component-model/ocm-project/issues/69 issue. So even if the ocm controller would consume the info the ocm cli doesn't let me set it.
The extra-identity is used by us. OCM the library isn't aware of such a thing. So specifying it there, does nothing. That's why you are getting that error.
ComponentVersion -> Localization ( helm chart ) -> Configuration ( helm chart ) -> FluxDeployer as mentioned in the description of this ticket.
The reason it doesn't work today is because this feature is incomplete as of now. Like I detailed in my comment, the Resource can produce a valid helm chart, but the others can't yet. :) Once it's implemented your scenario should work nicely. 🤞 😄
If ever helm starts insisting that the repo tag match the version inside Chart.yaml then things would break, and
The version is coming from here:
components:
- name: github.com/open-component-model/helm-test
version: "v1.0.0"
provider:
name: ocm.software
resources:
- name: charts
type: helmChart
version: 6.3.5
input:
type: helm
version: 6.3.5
path: charts/podinfo-6.3.5.tgz
So technically you can define whatever version you like when building the helm component using OCM the library.
Then, the resource uses that version as well when it's being reconciled:
apiVersion: delivery.ocm.software/v1alpha1
kind: Resource
metadata:
name: ocm-with-helm-deployment
namespace: ocm-system
spec:
interval: 10m
sourceRef:
kind: ComponentVersion
name: ocm-with-helm
namespace: ocm-system
resourceRef:
name: charts
version: 6.3.5
extraIdentity:
helmChart: podinfo
ComponentVersion -> Localization -> Resource -> Configuration
Hopefully, once everything is implemented, the Resource part could simply be left off. 🤞
Hey @Skarlso
First, let me say 'thank you' for taking the time to respond and having the patience deal with me. :)
"I assume you are talking about our local registry? "
Nope. I explained this above. Please see the screenshot above of artifactory. I circled the extra repository that was created and I explained why this is a problem.
"The extra-identity is used by us. OCM the library isn't aware of such a thing. So specifying it there, does nothing. That's why you are getting that error."
You must be able to see this makes zero sense from my perspective, yes? Consider what I am seeing as an outsider looking in
Regarding this example
components:
- name: github.com/open-component-model/helm-test
version: "v1.0.0"
provider:
name: ocm.software
resources:
- name: charts
type: helmChart
version: 6.3.5
input:
type: helm
version: 6.3.5
path: charts/podinfo-6.3.5.tgz
it is problematic for these reasons I mentioned above :
So again, from perspective, this should be an easy fix that doesn't break the model
Then in my component descriptor yaml I can put
name: dmi.csi.sap/dmi-multicloud-service-provisioner2
version: &version "1.0.74" # Doesn't seem to affect tags
provider:
name: dmi.csi.sap
resources:
- name: chart
type: dir
version: "1.0.75"
extraIdentity:
helmchart: "mychart"
input:
type: dir
path: ../helm/dmi-multicloud-service-provisioner
compress: true
preserveDir: true
and everything will 'just work' :)
Btw, the above is giving me the following in my componentdescripto, note the extraIdentity and version. So the information needed to do the right thing is there, just Localize and Configuration need to use it
componentReferences: []
name: dmi.csi.sap/dmi-multicloud-service-provisioner2
provider: dmi.csi.sap
repositoryContexts: []
resources:
- access:
localReference: sha256:645d3bf820707f82872a2766c423ea4303e4e4b462fa986429470e90420d247c
mediaType: application/x-tgz
type: localBlob
digest:
hashAlgorithm: SHA-256
normalisationAlgorithm: genericBlobDigest/v1
value: 645d3bf820707f82872a2766c423ea4303e4e4b462fa986429470e90420d247c
extraIdentity:
helmchart: mychart
name: chart
relation: local
type: dir
version: 1.0.75
Okay, so I think we have to make something clear. The controller is using OCM LIBRARY. Whatever the CLI does is the CLI's thing. :) The controller is using the Library.
So whatever the CLI does is not necessarily something the controller does. The controller has different goals and purposes.
This means that your component description doesn't need the extra identity the controller needs. Here: https://github.com/open-component-model/ocm-controller?tab=readme-ov-file#helmchart-type-resource it's explained that the controller is using this because it cannot be inferred. This is the only reason for it.
I could extend this of course if it doesn't make any sense. :)
Sorry, I might be dense, but where / what is creating that extra repo?
The chart is in a separate repo, which is something we don't want
But why?
So like I wrote, for now, configuration and localization isn't working with charts yet. I need to add that functionality. Resource though works with charts as long as OCM can fetch them. Whatever it creates internally, is not a concern of the user.
"what is creating that extra repo?"
The extra repository is created by the command
ocm --cred :type=OCIRegistry --cred :hostname=XXX --cred username=YYY --cred 'password=ZZZ' transfer component ./component-archive XXX/csi-ocm-poc
The chart is in a separate repo, which is something we don't want But why?
Because at least here, there is administrative overhead in having an additional repository that is used for production resources
Ahhhhh. I see. So ocm cli is creating it. I... can't do anything about that I'm afraid. :D
In the attached image I have circled in red the extra repo. The problem with the extra repo is that then we have to negotiate with the teams managing the various repositories we have to work with to request permission for 2 repositories instead of 1. Basically it will double the 'paperwork' involved in getting this to work.
Hi @dee0 , the first repository "component-descriptors" is created the first time a transfer of a component version to
component:
name: ocm.software/demos/echoserver
provider:
name: ocm.software
resources:
- access:
imageReference: ealen/echo-server:0.9.2
type: ociArtifact
name: image
relation: external
type: ociImage
version: 0.9.2
version: 0.9.2
Hey @morri-son
Thanks for chiming in.
No, I didn't use either of -V or --copy-resources. The command line I have been using is :
ocm --cred :type=OCIRegistry --cred :hostname=XXX --cred username=YYY --cred 'password=ZZZ' transfer component ./component-archive XXX/csi-ocm-poc
I believe this is now fixed with the recent move to OCI repos and a bit of a better handling of helm charts with removing the need to the name and tagging and everything.
@dee0 If you have the time, please re-test this. In the meantime, I'm gonna close it as fixed. Please re-open it if the problem persists. :)
Hey @Skarlso
Should I building and testing the main branch or should I test with the latest release?
Latest release should be okay now.
Hey @Skarlso
My first attempt to use the latest ocm-controller failed. I am still investigating however what I can tell you is :
Will provide updates as I get more data
Did you see any problems in the logs?
@Skarlso
Ok, for the cluster that hadn't reported a problem it turned out to fail the same way as the one did. There was quite some latency between the test completion and the results being collected.
So, first problem with updating to the ocm controller v0.23.7 is that it requires an update to the flux controllers. To deploy, I am using the output from ocm dry run after running it through a script that 'templatizes' it. Hopefully if I was using the cli to directly deploy or if I was using the new helm chart I would have received a warning about this.
Yesterday morning I upgraded all of the flux controllers to the latest release and kicked off another test. Looking at the results this morning I see that the upgrade has broken my POC. I haven't reviewed all the data yet, but the thing I see immediately is that the FluxDeployers aren't healthy. They have status like
status:
conditions:
- lastTransitionTime: "2024-06-24T17:48:35Z"
message: 'failed to create Helm Release object :failed to create reconcile kustomization: HelmRelease.helm.toolkit.fluxcd.io "dmi-broker" is invalid: [spec.chart.spec.sourceRef.name: Invalid value: "": spec.chart.spec.sourceRef.name in body should be at least 1 chars long, spec: Invalid value: "object": either chart or chartRef must be set]'
observedGeneration: 2
reason: CreateOrUpdateHelmFailed
status: "True"
type: Stalled
- lastTransitionTime: "2024-06-24T17:48:35Z"
message: 'failed to create Helm Release object :failed to create reconcile kustomization: HelmRelease.helm.toolkit.fluxcd.io "dmi-broker" is invalid: [spec.chart.spec.sourceRef.name: Invalid value: "": spec.chart.spec.sourceRef.name in body should be at least 1 chars long, spec: Invalid value: "object": either chart or chartRef must be set]'
observedGeneration: 2
reason: CreateOrUpdateHelmFailed
status: "False"
type: Ready
observedGeneration: 2
No idea why it is mentioning kustomizations. I am not using those at all.
That's a typo. It should be HelmRelease. What's your setup look like now? The new version updated the spec and the way we use HelmCharts to align with the new Flux version.
Here is a project that uses helm charts properly: https://github.com/Skarlso/ocm-helm
Component Version
apiVersion: delivery.ocm.software/v1alpha1
kind: ComponentVersion
metadata:
name: ocm-with-helm
namespace: ocm-system
spec:
interval: 10m0s
component: github.com/open-component-model/helm-test
version:
semver: 1.0.3
repository:
url: ghcr.io/skarlso/helm-test
secretRef:
name: creds
Resource:
apiVersion: delivery.ocm.software/v1alpha1
kind: Resource
metadata:
name: ocm-with-helm-deployment
namespace: ocm-system
spec:
interval: 10m
sourceRef:
kind: ComponentVersion
name: ocm-with-helm
namespace: ocm-system
resourceRef:
name: charts
version: 6.3.5
Deployer:
apiVersion: delivery.ocm.software/v1alpha1
kind: FluxDeployer
metadata:
name: fluxdeployer-podinfo-pipeline-backend
namespace: ocm-system
spec:
interval: 1m0s
sourceRef:
kind: Resource
name: ocm-with-helm-deployment
helmReleaseTemplate:
interval: 5m
To recap my setup, I have this setup
Here is one of my FluxDeployers
- apiVersion: delivery.ocm.software/v1alpha1
kind: FluxDeployer
metadata:
name: *name
namespace: *ocm-sys-namespace
spec:
interval: 10s
sourceRef:
kind: Configuration
name: *name
namespace: *namespace
helmReleaseTemplate:
timeout: 1h
chart:
spec:
chart: *name
interval: 10s
targetNamespace: *namespace
valuesFrom:
- kind: Secret
name: *landscapeSecretName
valuesKey: *secretKey
The Configuration status indicates they are healthy.
Another word about my setup...
For completeness sake, the FluxDeployer above was from a helm template. Here is the rendered result
- apiVersion: delivery.ocm.software/v1alpha1
kind: FluxDeployer
metadata:
creationTimestamp: "2024-06-24T17:46:20Z"
generation: 2
labels:
hc.sap.com/hc-provisioned: "True"
name: dmi-broker
namespace: ocm-system
resourceVersion: "19542"
uid: 5343f112-0df2-4c70-96d2-3dc3d8e7d41e
spec:
helmReleaseTemplate:
chart:
spec:
chart: dmi-broker
interval: 10s
targetNamespace: broker
timeout: 1h
valuesFrom:
- kind: Secret
name: dmi-broker-from-landscape
valuesKey: secret
interval: 10s
sourceRef:
apiVersion: delivery.ocm.software/v1alpha1
kind: Configuration
name: dmi-broker
namespace: broker
Hey @Skarlso At the moment I think what I am seeing is a new ocm-controller bug. It isn't filling in the helmReleaseTemplate sourceRef properly.
Gonna need some more info about your CRs. Or logs or something. Otherwise you're on your own.
Logs don't show really show anything more than what is in that status message. Attaching a zip of the ocm CRs ocm-crs-2024-06-25.zip
Each resource type is in its own *.yml file. Fwiw this is what our testing infrastructure is spitting out at the end of a failed test run.
Fwiw, this was working until the upgrade.
This:
resourceRef:
extraIdentity:
helmChart: dmi-broker
is no longer correct.
This is also no longer correct for the flux deployer:
helmReleaseTemplate:
chart:
spec:
chart: dmi-broker
Please check out the repository I linked. There is a working example for both helm chart without and with configuration. :)
Hey @Skarlso
Thanks, but I think your sample may be broken and I don't think it is representative for my case.
I think it is broken because the Localization appears to refer to a non-existent Resource. It refers to resource-pipeline-backend
and when I search for that the only hit is in localization.yaml.
As for why I don't think the samples are representative of my case there are these discrepancies
I am using both Localization and Configuration ( presumably this is normal ) but the examples don't use them in combination
My chart is a local resource. At the time when I was putting things together it didn't seem that I could localize or configure the chart's values.yaml so long as it was in a tgz file. So if that isn't the case using a tgz would be fine
resources:
- name: chart
version: *version
type: dir
input:
type: dir
path: "../helm/dmi-broker"
preserveDir: true
compress: true
In your examples the delivery.ocm.software resources are referring to specific chart versions. I can't do that. Whatever version happens to be in the ComponentVersion is what should be used.
Please provide some guidance on how to make this work with whatever change you made to the ocm-controller.
Ahhh sorry. I should have said that only configuration was altered to be working. The localization wasn't touched.
So sorry Dan.
I'll try to whip up a localization for you that's working. But generally I think it should once you alter it the same way configuration does it. Only the location of the image and tag needs to follow the value.yaml's format.
Thanks @Skarlso
Btw, the things I am concerned about are
Also is FluxDeployer able to work with a helm chart that is stored as a dir resource with in the component or must the type be helm chart? This is what I have been using so far ( successfully given the other files attached ) . Do I have to change this now? Or will the FluxDeployer not care as long as the content it is looking at actually appears to be a chart?
- name: chart
version: *version
type: dir
input:
type: dir
path: "../helm/dmi-broker"
preserveDir: true
compress: true
Hum. That shouldn't matter I think. Dir should work.
The requirement is there because it will try and produce a snapshot that the helm controller than needs to understand. And we do that previously already but that information isn't taken any further. Unless the resource is the same version as the chart than that extra info is not needed.
Hey @Skarlso just want to confirm what I think you said... In my component description if the resource version matches the helm chart version then I don't have to specify a version in the Resource or the FluxDeployer. So for example, in my component yaml if I have
name: dmi.csi.sap/dmi-broker
version: &version "1.0.0-1.badcafe"
provider:
name: dmi.csi.sap
resources:
- name: chart
version: 1.2.3 # <!-- Matches the helm chart version
type: dir
input:
type: dir
path: "../helm/dmi-broker"
preserveDir: true
compress: true
then I won't need to specify a chart version in the Resource or the FluxDeployer.
Do I have that correct?
Yeah, that should be right. As long as the tag in the snapshot matches the chart, Flux's helm controller shouldn't complain. 🤞
Nope, something went wrong. I'll dig some more, but what I am seeing is
kubectl --kubeconfig=kubeconfig-644-e1-dmi get -A helmrelease
NAMESPACE NAME AGE READY STATUS
ocm-system dmi-broker 29m False artifact revision 19090 does not match chart version 1.3.0-1085.xeae3fba.14
I have no idea where 19090 is coming from. Looking at the component descriptor with
kubectl --kubeconfig=kubeconfig-644-e1-dmi get -n broker componentdescriptor dmi.csi.sap-dmi-broker-1.3.0-1085.xeae3fba.14-17027996690744119944 -o=yaml
I see
spec:
resources:
- access:
localReference: sha256:4b942c25c3e772c826959cb4702a3e3d9f9b59be4a78449a15f7e5be4d5e4e12
mediaType: application/x-tgz
type: localBlob
digest:
hashAlgorithm: SHA-256
normalisationAlgorithm: genericBlobDigest/v1
value: 4b942c25c3e772c826959cb4702a3e3d9f9b59be4a78449a15f7e5be4d5e4e12
name: chart
relation: local
type: dir
version: 1.3.0-1085.xeae3fba.14
19090
is the observed revision which we are using by default as a tag. Hmm, the resource version should have overwritten that. For now, please use helmVersion instead. I'm on vacation for two weeks, so I'll look at it when I'm back.
I would like to have objects 'feeding' each other like so
ComponentVersion -> Localization -> Configuration -> FluxDeployer
Where
This doesn't work because Flux/Helm require that helm charts be in a namespaced repo instead of the being at the root of the registry. The helm-controller complains about a 404 due to the path it is looking for not existing.
Currently this can be worked around by changing the above to be ComponentVersion -> Localization -> Configuration -> Resource -> FluxDeployer where Resource has extraIdentity: helmChart: my-helm-chart-name
So the first question, why can't placement of extraIdentity on Configuration have the desired effect?
Second problem is, even after introducing a Resource as described above the deploy fails because the only tag in the repo is 'latest'.
Helm complains that no tags were found in the repo.
This is because Flux/helm require that the tags match a strict semver check and 'latest' doesn't pass this. I don't understand why the tag is 'latest' when my ComponentVersion is festooned with version info. Why can't one of those version numbers be utilized?
You can hack around this by adding a version spec on the resource like so sourceRef: kind: Configuration name: *name resourceRef: name: chart version: "1.0.37" Of course this isn't maintainable so this isn't a solution that one can live with.
And in any case, even adding the version still doesn't give you something that actually is usable. This because the Resource is picking up the helm chart from the ComponentVersion instad of the Snapshot created from the Configuration. Comparing resource_controller.go and localization_controller.go ( see the findObjects func ) provides an idea of why Resource is not producing the desired results.
I can try and work around all of this by adding
This is what the ocm demo here is doing. However the problem with this approach is that I am now assuming that ops is using Flux for deploying my service. I can accept the assumption that helm and ocm are in use but assuming Flux is too much.
Anyhoo as it stands at the processing of at least of the types Localization, Configuration or Resource needs to be changed in order to make it easier to consume helm charts as part of an ocm component.
I'll attach my component.yaml and the manifest file with the ComponentVersion etc.