-
Notifications
You must be signed in to change notification settings - Fork 24
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update firefly-helm-charts to FireFly 1.3 and KiND deployment #76
Conversation
Signed-off-by: Nicko Guyer <[email protected]>
Signed-off-by: Nicko Guyer <[email protected]>
Signed-off-by: Nicko Guyer <[email protected]>
Signed-off-by: Nicko Guyer <[email protected]>
Signed-off-by: Nicko Guyer <[email protected]>
Signed-off-by: Nicko Guyer <[email protected]>
Signed-off-by: Nicko Guyer <[email protected]>
Signed-off-by: Nicko Guyer <[email protected]>
Signed-off-by: Nicko Guyer <[email protected]>
Signed-off-by: Nicko Guyer <[email protected]>
Signed-off-by: Nicko Guyer <[email protected]>
Signed-off-by: Nicko Guyer <[email protected]>
Signed-off-by: Nicko Guyer <[email protected]>
Signed-off-by: SamMayWork <[email protected]>
Signed-off-by: SamMayWork <[email protected]>
Signed-off-by: SamMayWork <[email protected]>
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.
Looking good - quite a few tweaks needed
In discussion w/ @EnriqueL8 - there's a quite a few comments from the original PR around the app versions that we're using in these charts, this mostly appears to be leftovers from the original PR, but something that we need addressing. Problem: Right now we're not specifying a specific tag for the images that we're using for FireFly components, this means that we're defaulting to latest which might stop working at some point we don't know. The solution we're proposing is the following:
Automation to make the above easier is certainly possible and would be a meaningful contribution to this project. |
Signed-off-by: SamMayWork <[email protected]>
Signed-off-by: SamMayWork <[email protected]>
In reference to specific comments about things in the chart that aren't in the stack we're intending to use, for example AWS/Azure/Geth... It looks like parts of the values chart that we're using come from documentation from things like Besu and purposefully have multiple values so that they're unopinionated as possible, but not sure we want that here. Essentially, these helm charts now have 2 parts: deployment of chain infrastructure, and deployment of FireFly. For chain infrastructure we want to be very opinionated so that we don't break our KiND installation path, but our FireFly helm charts must be unopinionated. With that in mind, I'm going to make this PR very restrictive WRT which chain infrastructure we deploy. |
Signed-off-by: SamMayWork <[email protected]>
Signed-off-by: SamMayWork <[email protected]>
From comments here by @EnriqueL8 it looks like this PR being merged creates a few new items to make things here easier/more resilient:
I'll open these issues once this PR is merged |
Signed-off-by: SamMayWork <[email protected]>
OK - Calling this PR good for a re-review, it's worth noting that currently this is not going to work because of this issue hyperledger/firefly-dataexchange-https#81, once a release has been properly cut, I'll retest this PR and we should be good for a merge. |
I've just ran this locally
|
It would be nice if we could configure the swagger urls to be the port forward URL so that the swagger UI works with FireFly We exposed a configuration in FireFly to allow this https://hyperledger.github.io/firefly/latest/reference/config/#http We could also think about deploying an ingress instead of port-forwarding as it's such a common thing Update: already available in the |
Having a small tutorial on how to push to the kind registry a custom image would be amazing as well, doesn't need to be part of this PR |
Signed-off-by: SamMayWork <[email protected]>
We will hold off until 1.3.1 goes out and we will get this in! |
Signed-off-by: Andrew Richardson <[email protected]>
Update versions for FireFly v1.3.1
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.
Awesome, looks good to me now that the images have been updated
ref: #74
This PR is only a slight change from the core of the PR above with the following differences: