-
Notifications
You must be signed in to change notification settings - Fork 352
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
Add conditions to camel-k CRs #766
Conversation
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.
Like it!
cc05ef9
to
66c8e5c
Compare
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.
Love it!
Two general comments:
- Currently, conditions are shared among CRs, so it is possible to mix them, I wonder whether there should be dedicated conditions types per CR
- It seems there is a larger need to report the owned resources and I wonder to what extend it generalises the proposed
DeploymentAvailable
andExposureAvailable
conditions
Yes, that’s what I’m working on. It is a little bit of a pain as we basically need to duplicate all the code but that’t the way to go :)
Yes, the example here is just a placeholder to show how it will look but the final goal is to report any owned resource |
08742f4
to
e67fc49
Compare
So I've added a little bit more logic and now if you describe an integration while it is building a kit you'll see something like: Name: routes-rest
Namespace: myproject
Creation Timestamp: Mon, 08 Jul 2019 13:34:02 +0200
Phase: Building Kit
Camel Version: 3.0.0-M3
Kit: kit-bkhikar8gd7uoo13f47g
Image:
Version: 1.0.0-M1-SNAPSHOT
Dependencies:
camel:core
camel:log
camel:timer
camel:undertow
runtime:jvm
Sources:
Name Language Compression Ref Ref Key
routes-rest.js js false
Conditions:
Type Status Reason Message
IntegrationPlatformAvailable True IntegrationPlatformAvailable camel-k
IntegrationKitAvailable False IntegrationKitAvailable Once the kit is ready, then the Name: routes-rest
Namespace: myproject
Creation Timestamp: Mon, 08 Jul 2019 13:34:02 +0200
Phase: Running
Camel Version: 3.0.0-M3
Kit: kit-bkhikar8gd7uoo13f47g
Image: 172.30.1.1:5000/myproject/camel-k-kit-bkhikar8gd7uoo13f47g:1281056
Version: 1.0.0-M1-SNAPSHOT
Dependencies:
camel:core
camel:log
camel:timer
camel:undertow
runtime:jvm
Sources:
Name Language Compression Ref Ref Key
routes-rest.js js false
Conditions:
Type Status Reason Message
IntegrationPlatformAvailable True IntegrationPlatformAvailable camel-k
IntegrationKitAvailable True IntegrationKitAvailable kit-bkhikar8gd7uoo13f47g
DeploymentAvailable True DeploymentAvailable routes-rest
ExposureAvailable True RouteAvailable routes-rest -> routes-rest(http)
ServiceAvailable True ServiceAvailable routes-rest(http/80) -> routes-rest(http/8080) If you run the same integration but with the Name: routes-rest
Namespace: myproject
Creation Timestamp: Mon, 08 Jul 2019 13:37:12 +0200
Phase: Running
Camel Version: 3.0.0-M3
Kit: kit-bkhikar8gd7uoo13f47g
Image: 172.30.1.1:5000/myproject/camel-k-kit-bkhikar8gd7uoo13f47g:1281056
Version: 1.0.0-M1-SNAPSHOT
Dependencies:
camel:core
camel:log
camel:timer
camel:undertow
runtime:jvm
Sources:
Name Language Compression Ref Ref Key
routes-rest.js js false
Conditions:
Type Status Reason Message
IntegrationPlatformAvailable True IntegrationPlatformAvailable camel-k
ServiceAvailable False ServiceNotAvailable explicitly disabled
IntegrationKitAvailable True IntegrationKitAvailable kit-bkhikar8gd7uoo13f47g
DeploymentAvailable True DeploymentAvailable routes-rest
ExposureAvailable False RouteNotAvailable no target service found |
@astefanutti @nicolaferraro we still miss to additional information such as the config maps that the integration creates/requires among the conditions but didn't want to make the PR too big, can you please review ? |
@@ -78,6 +79,14 @@ func add(mgr manager.Manager, r reconcile.Reconciler) error { | |||
UpdateFunc: func(e event.UpdateEvent) bool { | |||
oldIntegration := e.ObjectOld.(*v1alpha1.Integration) | |||
newIntegration := e.ObjectNew.(*v1alpha1.Integration) | |||
|
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.
My understanding is that this statement is already logged in the Reconcile
method.
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.
oh yeah, that's a leftover from the rebase, will fix it
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.
done
I wonder if it'd be possible to have a base type for the conditions that would enable factorising some of the helper methods as well. |
yeah, who needs generics ? :P |
Let's wait for go to have generics 😄 |
@astefanutti can this be merged ? |
Good for me 😉 |
Fixes #594
I did a little bit of work to introduce
conditions
in camel-k CRs so now if you invokeyou'd get an output like:
which includes a number of
conditions
that represents the latest available observations of a integration state.The goal here is to provide more insight about the integration status i.e. the additional resources associated to the integration that the operator has created (deployment vs knative service, service/route/ingress, etc) so user and tools do not need to explore the integration namespace to know what are the other resources related to the integration.
@astefanutti @nicolaferraro does this make sense ?