You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by GitBox <gi...@apache.org> on 2021/05/18 13:34:12 UTC
[GitHub] [camel-k] squakez opened a new pull request #2309: feat(cmd): build-property flag
squakez opened a new pull request #2309:
URL: https://github.com/apache/camel-k/pull/2309
<!-- Description -->
With this PR we are introducing a new parameter, `--build-property`, whose goal is to let the user specify any configuration needed at build time. For example:
```
kamel run --build-property=quarkus.application.name=my-super-application ...
```
Will provide the `quarkus.application.name` to the properties expected by the build tasks, setting, in this case, the name of the application as expected by `camel-quarkus`.
Ref #2003
<!--
Enter your extended release note in the below block. If the PR requires
additional action from users switching to the new release, include the string
"action required". If no release note is required, write "NONE".
You can (optionally) mark this PR with labels "kind/bug" or "kind/feature" to make sure
the text is added to the right section of the release notes.
-->
**Release Note**
```release-note
feat(cmd): build-property flag
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] squakez commented on a change in pull request #2309: feat(cmd): build-property flag
Posted by GitBox <gi...@apache.org>.
squakez commented on a change in pull request #2309:
URL: https://github.com/apache/camel-k/pull/2309#discussion_r635923164
##########
File path: pkg/trait/builder.go
##########
@@ -59,7 +63,11 @@ func (t *builderTrait) Configure(e *Environment) (bool, error) {
}
func (t *builderTrait) Apply(e *Environment) error {
- builderTask := t.builderTask(e)
+ builderTask, err := t.builderTask(e)
+ if err != nil {
+ // Should we clean the failing integration kit as well?
Review comment:
Thanks. I managed to fix the problem by setting an error status:
```
e.IntegrationKit.Status.Phase = v1.IntegrationKitPhaseError
e.IntegrationKit.Status.SetCondition("IntegrationKitPropertiesFormatValid", corev1.ConditionFalse,
"IntegrationKitPropertiesFormatValid", fmt.Sprintf("One or more properties where not formatted as expected: %s", err.Error()))
return nil
```
However, after 5 retries, I'd expect to see the condition with the error specified in the `IntegrationKit`, but I just see a generic error:
```
status:
conditions:
- lastTransitionTime: "2021-05-20T09:07:43Z"
lastUpdateTime: "2021-05-20T09:07:43Z"
message: IntegrationPlatform (camel-k)
reason: IntegrationPlatformAvailable
status: "True"
type: IntegrationPlatformAvailable
digest: v-xWT3kPLcPJ2Lhu0eFac7eWDCCIZBCbZhIPQRWPRBxQ
failure:
reason: ""
recovery:
attempt: 5
attemptMax: 5
attemptTime: "2021-05-20T09:10:04Z"
time: "2021-05-20T09:07:48Z"
phase: Error
```
How can I provide that error message in the `IntegrationKit`. I think we must set the failure reason to let the user know why it failed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] squakez commented on a change in pull request #2309: feat(cmd): build-property flag
Posted by GitBox <gi...@apache.org>.
squakez commented on a change in pull request #2309:
URL: https://github.com/apache/camel-k/pull/2309#discussion_r639513315
##########
File path: pkg/trait/builder.go
##########
@@ -59,7 +63,11 @@ func (t *builderTrait) Configure(e *Environment) (bool, error) {
}
func (t *builderTrait) Apply(e *Environment) error {
- builderTask := t.builderTask(e)
+ builderTask, err := t.builderTask(e)
+ if err != nil {
+ // Should we clean the failing integration kit as well?
Review comment:
Nevermind, I've just realized I was forgetting to update the status. Fixed with last commit.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] astefanutti commented on a change in pull request #2309: feat(cmd): build-property flag
Posted by GitBox <gi...@apache.org>.
astefanutti commented on a change in pull request #2309:
URL: https://github.com/apache/camel-k/pull/2309#discussion_r634392785
##########
File path: pkg/trait/builder.go
##########
@@ -32,6 +34,8 @@ type builderTrait struct {
BaseTrait `property:",squash"`
// Enable verbose logging on build components that support it (e.g. Kaniko build pod).
Verbose *bool `property:"verbose" json:"verbose,omitempty"`
+ // A list of properties to be provided to the build task
+ BuildTimeProperties []string `property:"build-time-properties" json:"buildTimeProperties,omitempty"`
Review comment:
Can I suggest:
Properties []string `property:"properties" json:"Properties,omitempty"`
It's quite clear in the scope of the builder trait that these properties are applied at build time.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] squakez commented on a change in pull request #2309: feat(cmd): build-property flag
Posted by GitBox <gi...@apache.org>.
squakez commented on a change in pull request #2309:
URL: https://github.com/apache/camel-k/pull/2309#discussion_r634387536
##########
File path: pkg/trait/builder.go
##########
@@ -59,7 +63,11 @@ func (t *builderTrait) Configure(e *Environment) (bool, error) {
}
func (t *builderTrait) Apply(e *Environment) error {
- builderTask := t.builderTask(e)
+ builderTask, err := t.builderTask(e)
+ if err != nil {
+ // Should we clean the failing integration kit as well?
Review comment:
Ì have a doubt here. If the task fails (ie, a bad format property is provided), we are safely returning and showing a log on the operator side. However, the failing integration kit will hang around until a manual removal is performed. I wonder if in this situation we can mark the `IntegrationKit` in order to be cleaned. Is that possible? how to perform that? @nicolaferraro @astefanutti do you have any suggestion?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] nicolaferraro merged pull request #2309: feat(cmd): build-property flag
Posted by GitBox <gi...@apache.org>.
nicolaferraro merged pull request #2309:
URL: https://github.com/apache/camel-k/pull/2309
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] astefanutti commented on a change in pull request #2309: feat(cmd): build-property flag
Posted by GitBox <gi...@apache.org>.
astefanutti commented on a change in pull request #2309:
URL: https://github.com/apache/camel-k/pull/2309#discussion_r634392785
##########
File path: pkg/trait/builder.go
##########
@@ -32,6 +34,8 @@ type builderTrait struct {
BaseTrait `property:",squash"`
// Enable verbose logging on build components that support it (e.g. Kaniko build pod).
Verbose *bool `property:"verbose" json:"verbose,omitempty"`
+ // A list of properties to be provided to the build task
+ BuildTimeProperties []string `property:"build-time-properties" json:"buildTimeProperties,omitempty"`
Review comment:
Can I suggest:
```go
Properties []string `property:"properties" json:"properties,omitempty"`
```
It's quite clear in the scope of the builder trait that these properties are applied at build time.
Or possibly:
```go
MavenProperties []string `property:"maven-properties" json:"mavenProperties,omitempty"`
```
To make it clearer that these are Maven properties.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] astefanutti commented on a change in pull request #2309: feat(cmd): build-property flag
Posted by GitBox <gi...@apache.org>.
astefanutti commented on a change in pull request #2309:
URL: https://github.com/apache/camel-k/pull/2309#discussion_r634407932
##########
File path: pkg/trait/builder.go
##########
@@ -59,7 +63,11 @@ func (t *builderTrait) Configure(e *Environment) (bool, error) {
}
func (t *builderTrait) Apply(e *Environment) error {
- builderTask := t.builderTask(e)
+ builderTask, err := t.builderTask(e)
+ if err != nil {
+ // Should we clean the failing integration kit as well?
Review comment:
By default, an event will be emitted:
https://github.com/apache/camel-k/blob/1b64912e2f1e67bb3dfe287d3ba65ddc5e50cf33/pkg/controller/integrationkit/integrationkit_controller.go#L252
In that case, the reconciliation request is requeued for retry. That is applicable to general errors, that may be recoverable, such as conflicting changes. In case of an error that we know cannot be recovered, the IntegrationKit phase must be set to `IntegrationKitPhaseError`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] squakez commented on a change in pull request #2309: feat(cmd): build-property flag
Posted by GitBox <gi...@apache.org>.
squakez commented on a change in pull request #2309:
URL: https://github.com/apache/camel-k/pull/2309#discussion_r635923164
##########
File path: pkg/trait/builder.go
##########
@@ -59,7 +63,11 @@ func (t *builderTrait) Configure(e *Environment) (bool, error) {
}
func (t *builderTrait) Apply(e *Environment) error {
- builderTask := t.builderTask(e)
+ builderTask, err := t.builderTask(e)
+ if err != nil {
+ // Should we clean the failing integration kit as well?
Review comment:
Thanks. I managed to fix the problem by setting an error status:
```
e.IntegrationKit.Status.Phase = v1.IntegrationKitPhaseError
e.IntegrationKit.Status.SetCondition("IntegrationKitPropertiesFormatValid", corev1.ConditionFalse,
"IntegrationKitPropertiesFormatValid", fmt.Sprintf("One or more properties where not formatted as expected: %s", err.Error()))
return nil
```
However, after 5 retries, I'd expect to see the condition with the error specified in the `IntegrationKit`, but I just see a generic error:
```
status:
conditions:
- lastTransitionTime: "2021-05-20T09:07:43Z"
lastUpdateTime: "2021-05-20T09:07:43Z"
message: IntegrationPlatform (camel-k)
reason: IntegrationPlatformAvailable
status: "True"
type: IntegrationPlatformAvailable
digest: v-xWT3kPLcPJ2Lhu0eFac7eWDCCIZBCbZhIPQRWPRBxQ
failure:
reason: ""
recovery:
attempt: 5
attemptMax: 5
attemptTime: "2021-05-20T09:10:04Z"
time: "2021-05-20T09:07:48Z"
phase: Error
```
How can I provide that error message in the `IntegrationKit`?. I think we must set the failure reason to let the user know why it failed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] astefanutti commented on a change in pull request #2309: feat(cmd): build-property flag
Posted by GitBox <gi...@apache.org>.
astefanutti commented on a change in pull request #2309:
URL: https://github.com/apache/camel-k/pull/2309#discussion_r634392785
##########
File path: pkg/trait/builder.go
##########
@@ -32,6 +34,8 @@ type builderTrait struct {
BaseTrait `property:",squash"`
// Enable verbose logging on build components that support it (e.g. Kaniko build pod).
Verbose *bool `property:"verbose" json:"verbose,omitempty"`
+ // A list of properties to be provided to the build task
+ BuildTimeProperties []string `property:"build-time-properties" json:"buildTimeProperties,omitempty"`
Review comment:
Can I suggest:
Properties []string `property:"properties" json:"properties,omitempty"`
It's quite clear in the scope of the builder trait that these properties are applied at build time.
Or possibly:
MavenProperties []string `property:"maven-properties" json:"mavenProperties,omitempty"`
To make it clearer that these are Maven properties.
##########
File path: pkg/trait/builder.go
##########
@@ -32,6 +34,8 @@ type builderTrait struct {
BaseTrait `property:",squash"`
// Enable verbose logging on build components that support it (e.g. Kaniko build pod).
Verbose *bool `property:"verbose" json:"verbose,omitempty"`
+ // A list of properties to be provided to the build task
+ BuildTimeProperties []string `property:"build-time-properties" json:"buildTimeProperties,omitempty"`
Review comment:
Can I suggest:
Properties []string `property:"properties" json:"properties,omitempty"`
It's quite clear in the scope of the builder trait that these properties are applied at build time.
Or possibly:
MavenProperties []string `property:"maven-properties" json:"mavenProperties,omitempty"`
To make it clearer that these are Maven properties.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org