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/11/23 11:36:38 UTC
[GitHub] [camel-k] squakez opened a new pull request #2771: feat(cmd/run): autogenerated configmap for resource/config local files
squakez opened a new pull request #2771:
URL: https://github.com/apache/camel-k/pull/2771
With this draft PR I've run some experiment to check how we can remove the `ResourceSpec` from the `IntegrationSpec` and still be able to let the user provide a local file as a resource.
The idea is that the CLI will take care to parse a file, autogenerate to a `Configmap` and bind it to the `Integration` as it would be any other configmap. The resource will be owned by the Integration, so, deleted as soon as the Integration will be deleted. We also watch for resource file change to allow `--sync` option and regenerate the configmap upon a file change detection.
With this approach (source code to be polished more) we can still keep the possibility for the user to provide a file and at the same time we can remove the need to include the resource content in the `Integration`. If we like this approach we can extend it to the `--openapi` and definetely remove the `ResourceSpec`.
Ref #2320
<!-- Description -->
<!--
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
NONE
```
--
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.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] astefanutti commented on a change in pull request #2771: feat(cmd/run): autogenerated configmap for resource/config local files
Posted by GitBox <gi...@apache.org>.
astefanutti commented on a change in pull request #2771:
URL: https://github.com/apache/camel-k/pull/2771#discussion_r777990928
##########
File path: pkg/trait/mount.go
##########
@@ -0,0 +1,177 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package trait
+
+import (
+ "fmt"
+ "strings"
+
+ appsv1 "k8s.io/api/apps/v1"
+ "k8s.io/api/batch/v1beta1"
+ corev1 "k8s.io/api/core/v1"
+
+ serving "knative.dev/serving/pkg/apis/serving/v1"
+
+ v1 "github.com/apache/camel-k/pkg/apis/camel/v1"
+ "github.com/apache/camel-k/pkg/util/kubernetes"
+ utilResource "github.com/apache/camel-k/pkg/util/resource"
+)
+
+// The Mount trait can be used to configure volumes mounted on the Integration Pod.
+//
+// +camel-k:trait=mount
+// nolint: tagliatelle
+type mountTrait struct {
+ BaseTrait `property:",squash"`
+ // A list of configuration pointing to configmap/secret. Syntax: [configmap|secret]:name[key], where name represents the resource name and key optionally represents the resource key to be filtered
+ Configs []string `property:"configs" json:"configs,omitempty"`
Review comment:
The comment now documents what kind of content can be provided. I could still imagine an external user asking what's the difference between configs and resources functionally. May I suggest to add that configs are added to the classpath, and that they are typically used to configure Camel.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] astefanutti commented on pull request #2771: feat(cmd/run): autogenerated configmap for resource/config local files
Posted by GitBox <gi...@apache.org>.
astefanutti commented on pull request #2771:
URL: https://github.com/apache/camel-k/pull/2771#issuecomment-1007414198
> @astefanutti any last look before I can merge?
@squakez LGTM 👍🏼 Great work!
--
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.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] astefanutti commented on a change in pull request #2771: feat(cmd/run): autogenerated configmap for resource/config local files
Posted by GitBox <gi...@apache.org>.
astefanutti commented on a change in pull request #2771:
URL: https://github.com/apache/camel-k/pull/2771#discussion_r778037581
##########
File path: pkg/trait/mount.go
##########
@@ -0,0 +1,199 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package trait
+
+import (
+ "fmt"
+ "strings"
+
+ appsv1 "k8s.io/api/apps/v1"
+ "k8s.io/api/batch/v1beta1"
+ corev1 "k8s.io/api/core/v1"
+
+ serving "knative.dev/serving/pkg/apis/serving/v1"
+
+ v1 "github.com/apache/camel-k/pkg/apis/camel/v1"
+ "github.com/apache/camel-k/pkg/util/kubernetes"
+ utilResource "github.com/apache/camel-k/pkg/util/resource"
+)
+
+// The Mount trait can be used to configure volumes mounted on the Integration Pods.
+//
+// +camel-k:trait=mount
+// nolint: tagliatelle
+type mountTrait struct {
+ BaseTrait `property:",squash"`
+ // A list of configuration pointing to configmap/secret.
+ // The configuration are expected to be ÙTF-8 resources as they are processed by runtime Camel Context and tried to be parsed as property files.
Review comment:
nit: `ÙTF-8` -> `UTF-8`
--
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.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] astefanutti commented on pull request #2771: feat(cmd/run): autogenerated configmap for resource/config local files
Posted by GitBox <gi...@apache.org>.
astefanutti commented on pull request #2771:
URL: https://github.com/apache/camel-k/pull/2771#issuecomment-999507925
> > > Thanks a lot for the review @astefanutti , it's a long one, I really appreciate your help with this.
> > > > My understanding is that the implementation assumes the "local file" use case is different than the "standard" ConfigMap use case. It's possible you've already tried to explain the difference, though could you please clarify the rational behind that assumption. It seems that the file prefix / reference gets stored in the container trait configuration, and even if it's weakly linked to the local file, and resolved to the generated ConfigMap, that seems to break the "standalone" configuration principle
> > >
> > >
> > > The idea we discussed was to go in the direction to [remove any arbitrary file content from the integration](https://github.com/apache/camel-k/issues/2320#issuecomment-922718785). With this PR what we do is to automatically help the user convert a local file into a configmap, and then attach to the Integration. In my opinion what we're doing is to remove the support for local file, although we help the user to convert it in the supported storage via `kamel` CLI to simplify his life. If the user is not able to run an Integration via `kamel run`, in fact, the feature cannot be provided.
> >
> >
> > That is my background too. My question was more, why does the `file` prefix beyond the CLI, and the generated ConfigMap seems to be treated differently than the other "standard" ones?
>
> If I understood correctly, the question is about generated Configmap difference with user provided Configmap. The only difference is about the lifecycle of the Configmap itself, as my idea is to have it living together and exclusively within the Integration lifecycle. This is mimicking the previous behavior, when a Configmap was generated beside the Integration. The difference is that the resource is not directly stored in the Integration and the Configmap is generated by the CLI (as it must access the file content). Another possibility would be to autogenerate the Configmap but let the lifecycle (update/delete) up to the user. However, I think it completely overlaps what we already provide with the normal configmap, just saving the manual step to create the configmap from the file resource.
>
> Hope it answer your question, I'm not entirely sure we're talking about the same thing here :)
I think we are talking about the same thing :) But that is general description, that I still fail to reconcile with the actual implementation :) Maybe this could be cleared if you could help me understand:
* Why the file information leaks into the _container_ trait configuration, following the comment ` Syntax: [configmap|secret|file]:name[key], where name represents the local file path or the configmap/secret` and my interpretation of the current implementation?
* Also why the generated ConfigMap name is hashed based on local information? Could we mimic what `kubectl create configmap` does?
I'm trying to understand if the consistency of the Integration configuration being created is still satisfied, meaning if user A create an integration with a local file resource, user B should still be able to operate the Integration. I hope that makes sense :)
> > > > The generated ConfigMap is owned by the Integration, which makes the former gets garbage collected when the former is deleted. However, how is it garbage collected when the Integration is updated with the file resource removed, for example with a new kamel run invocation, without the resource?
> > >
> > >
> > > Good point, I did not consider this situation. The configmap is updated when there is a file change and `--sync/--dev` enabled, but I did not check what happens when the file is removed at all. I'll run some test and come back to this point.
>
> I made some test and with this implementation, the generated Configmap is kept until the Integration is living. We can provide an additional loop to reconcile the generated Configmap not needed right after the binding step. Or we can keep it the way it is, if we consider the pattern to create an Integration with a file and later update the same integration without a file as a rare one. I don't have a strong opinion on this part.
What I'm trying to determine here, aren't we about to duplicate the garbage collection that we already have for resources generated by the operator. I get the particularity of the "local file" use case, but would there be a way to make it fit into the existing garbage collection mechanism.
> > > > What's the rational, from the functional standpoint, to use the container trait to host the configuration resources? It's already responsible for the compute resources, the container image, the service, and the (deprecated) health probes configuration.
> > >
> > >
> > > The container trait is already in charge to convert the configmaps/secret/... into mounted volumes ([we discussed it shortly as well](https://github.com/apache/camel-k/pull/2635#issuecomment-922720383)), so it felt the most appropriate trait to use. Otherwise we need somehow to move that logic elsewhere or duplicating it, which I don't think is a good idea.
> >
> >
> > I'm trying to reason first from the end-user standpoint. The CLI hides it, still it's part of the API. I remember we had the discussion in #2635, and I was wondering whether that was the right moment to find a more "functionally oriented" trait to capture this concern, and disentangle the _container_ trait.
>
> The idea to move the _volumes_ logic into a separate trait could be interesting. However, we need to attach those volumes into the `corev1.Container`. If we create a new trait, then it will be very dependent on the container trait, requiring to be executed after it, and it will need to get the `corev1.Container` spec. After all, I think it is responsibility of the container trait to manage its own volumes.
Yes, that is also my understanding of the technical details. I'm trying to reason at the functional level, from the end-user standpoint.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] squakez edited a comment on pull request #2771: feat(cmd/run): autogenerated configmap for resource/config local files
Posted by GitBox <gi...@apache.org>.
squakez edited a comment on pull request #2771:
URL: https://github.com/apache/camel-k/pull/2771#issuecomment-999488606
> > Thanks a lot for the review @astefanutti , it's a long one, I really appreciate your help with this.
> > > My understanding is that the implementation assumes the "local file" use case is different than the "standard" ConfigMap use case. It's possible you've already tried to explain the difference, though could you please clarify the rational behind that assumption. It seems that the file prefix / reference gets stored in the container trait configuration, and even if it's weakly linked to the local file, and resolved to the generated ConfigMap, that seems to break the "standalone" configuration principle
> >
> >
> > The idea we discussed was to go in the direction to [remove any arbitrary file content from the integration](https://github.com/apache/camel-k/issues/2320#issuecomment-922718785). With this PR what we do is to automatically help the user convert a local file into a configmap, and then attach to the Integration. In my opinion what we're doing is to remove the support for local file, although we help the user to convert it in the supported storage via `kamel` CLI to simplify his life. If the user is not able to run an Integration via `kamel run`, in fact, the feature cannot be provided.
>
> That is my background too. My question was more, why does the `file` prefix beyond the CLI, and the generated ConfigMap seems to be treated differently than the other "standard" ones?
If I understood correctly, the question is about generated Configmap difference with user provided Configmap. The only difference is about the lifecycle of the Configmap itself, as my idea is to have it living together and exclusively within the Integration lifecycle. This is mimicking the previous behavior, when a Configmap was generated beside the Integration. The difference is that the resource is not directly stored in the Integration and the Configmap is generated by the CLI (as it must access the file content). Another possibility would be to autogenerate the Configmap but let the lifecycle (update/delete) up to the user. However, I think it completely overlaps what we already provide with the normal configmap, just saving the manual step to create the configmap from the file resource.
Hope it answer your question, I'm not entirely sure we're talking about the same thing here :)
>
> > > The generated ConfigMap is owned by the Integration, which makes the former gets garbage collected when the former is deleted. However, how is it garbage collected when the Integration is updated with the file resource removed, for example with a new kamel run invocation, without the resource?
> >
> >
> > Good point, I did not consider this situation. The configmap is updated when there is a file change and `--sync/--dev` enabled, but I did not check what happens when the file is removed at all. I'll run some test and come back to this point.
I made some test and with this implementation, the generated Configmap is kept until the Integration is living. We can provide an additional loop to reconcile the generated Configmap not needed right after the binding step. Or we can keep it the way it is, if we consider the pattern to create an Integration with a file and later update the same integration without a file as a rare one. I don't have a strong opinion on this part.
> > > What's the rational, from the functional standpoint, to use the container trait to host the configuration resources? It's already responsible for the compute resources, the container image, the service, and the (deprecated) health probes configuration.
> >
> >
> > The container trait is already in charge to convert the configmaps/secret/... into mounted volumes ([we discussed it shortly as well](https://github.com/apache/camel-k/pull/2635#issuecomment-922720383)), so it felt the most appropriate trait to use. Otherwise we need somehow to move that logic elsewhere or duplicating it, which I don't think is a good idea.
>
> I'm trying to reason first from the end-user standpoint. The CLI hides it, still it's part of the API. I remember we had the discussion in #2635, and I was wondering whether that was the right moment to find a more "functionally oriented" trait to capture this concern, and disentangle the _container_ trait.
The idea to move the _volumes_ logic into a separate trait could be interesting. However, we need to attach those volumes into the `corev1.Container`. If we create a new trait, then it will be very dependent on the container trait, requiring to be executed after it, and it will need to get the `corev1.Container` spec. After all, I think it is responsibility of the container trait to manage its own volumes.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] squakez commented on pull request #2771: feat(cmd/run): autogenerated configmap for resource/config local files
Posted by GitBox <gi...@apache.org>.
squakez commented on pull request #2771:
URL: https://github.com/apache/camel-k/pull/2771#issuecomment-1002132432
I've finished another iteration of development in order to address the points discussed previously. With d85257c we're now validating the input submitted to the trait: we won't allow anything but `configmap` or `secret`. With 78e62f4 we're introducing a new trait, called `mount` which can be used to _mount_ volumes from the different resources we allow.
I prefer this name to a generic `resource` as it declare exactly its purpose.
The last outstanding point is about garbage collecting the autogenerated configmaps that are used and later removed. Right now they will live beside the Integration, until this one is deleted. I cannot figure out how to manage differently and let them be garbage collected as soon as they are updated (unless we do a manual clean). IMO we can keep them along.
@astefanutti please, have a further look and let me know what do you think.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] astefanutti commented on pull request #2771: feat(cmd/run): autogenerated configmap for resource/config local files
Posted by GitBox <gi...@apache.org>.
astefanutti commented on pull request #2771:
URL: https://github.com/apache/camel-k/pull/2771#issuecomment-1003988144
> I've finished another iteration of development in order to address the points discussed previously. With [d85257c](https://github.com/apache/camel-k/commit/d85257c2e180826df677b472da5a484093ae8b69) we're now validating the input submitted to the trait: we won't allow anything but `configmap` or `secret`. With [78e62f4](https://github.com/apache/camel-k/commit/78e62f4742bc1347218496a3be09348c979b7608) we're introducing a new trait, called `mount` which can be used to _mount_ volumes from the different resources we allow.
>
> I prefer this name to a generic `resource` as it declare exactly its purpose.
It sounds and looks great to me 👍🏼.
> The last outstanding point is about garbage collecting the autogenerated configmaps that are used and later removed. Right now they will live beside the Integration, until this one is deleted. I cannot figure out how to manage differently and let them be garbage collected as soon as they are updated (unless we do a manual clean). IMO we can keep them along.
What I'm thinking is a process like this:
1. The CLI creates the ConfigMap with the `camel.apache.org/generated` annotation, and does only that
2. The _mount_ trait adds the ConfigMap to the Integration resources, by iterating over its configured ConfigMaps, and filtering those annotated with `camel.apache.org/generated`
3. The _owner_ trait sets ownership on the ConfigMap OOTB
4. The _gc_ trait associates the ConfigMap to the Integration current generation, and eventually garbages it OOTB
--
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.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] squakez merged pull request #2771: feat(cmd/run): autogenerated configmap for resource/config local files
Posted by GitBox <gi...@apache.org>.
squakez merged pull request #2771:
URL: https://github.com/apache/camel-k/pull/2771
--
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.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] astefanutti commented on a change in pull request #2771: feat(cmd/run): autogenerated configmap for resource/config local files
Posted by GitBox <gi...@apache.org>.
astefanutti commented on a change in pull request #2771:
URL: https://github.com/apache/camel-k/pull/2771#discussion_r777379979
##########
File path: pkg/util/kubernetes/factory.go
##########
@@ -114,3 +115,34 @@ func NewResourceRequirements(reqs []string) (corev1.ResourceRequirements, error)
return resReq, nil
}
+
+// NewConfigmap will create a Configmap.
+func NewConfigmap(namespace, cmName, originalFilename string, generatedKey string,
Review comment:
nit: NewConfigmap -> NewConfigMap
##########
File path: pkg/trait/mount.go
##########
@@ -0,0 +1,177 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package trait
+
+import (
+ "fmt"
+ "strings"
+
+ appsv1 "k8s.io/api/apps/v1"
+ "k8s.io/api/batch/v1beta1"
+ corev1 "k8s.io/api/core/v1"
+
+ serving "knative.dev/serving/pkg/apis/serving/v1"
+
+ v1 "github.com/apache/camel-k/pkg/apis/camel/v1"
+ "github.com/apache/camel-k/pkg/util/kubernetes"
+ utilResource "github.com/apache/camel-k/pkg/util/resource"
+)
+
+// The Mount trait can be used to configure volumes mounted on the Integration Pod.
Review comment:
nit: Pod -> Pod(s)
##########
File path: pkg/trait/mount.go
##########
@@ -0,0 +1,177 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package trait
+
+import (
+ "fmt"
+ "strings"
+
+ appsv1 "k8s.io/api/apps/v1"
+ "k8s.io/api/batch/v1beta1"
+ corev1 "k8s.io/api/core/v1"
+
+ serving "knative.dev/serving/pkg/apis/serving/v1"
+
+ v1 "github.com/apache/camel-k/pkg/apis/camel/v1"
+ "github.com/apache/camel-k/pkg/util/kubernetes"
+ utilResource "github.com/apache/camel-k/pkg/util/resource"
+)
+
+// The Mount trait can be used to configure volumes mounted on the Integration Pod.
+//
+// +camel-k:trait=mount
+// nolint: tagliatelle
+type mountTrait struct {
+ BaseTrait `property:",squash"`
+ // A list of configuration pointing to configmap/secret. Syntax: [configmap|secret]:name[key], where name represents the resource name and key optionally represents the resource key to be filtered
+ Configs []string `property:"configs" json:"configs,omitempty"`
Review comment:
It may be useful to state in what `Configs` and `Resources` differ.
##########
File path: pkg/trait/mount.go
##########
@@ -0,0 +1,177 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package trait
+
+import (
+ "fmt"
+ "strings"
+
+ appsv1 "k8s.io/api/apps/v1"
+ "k8s.io/api/batch/v1beta1"
+ corev1 "k8s.io/api/core/v1"
+
+ serving "knative.dev/serving/pkg/apis/serving/v1"
+
+ v1 "github.com/apache/camel-k/pkg/apis/camel/v1"
+ "github.com/apache/camel-k/pkg/util/kubernetes"
+ utilResource "github.com/apache/camel-k/pkg/util/resource"
+)
+
+// The Mount trait can be used to configure volumes mounted on the Integration Pod.
+//
+// +camel-k:trait=mount
+// nolint: tagliatelle
+type mountTrait struct {
+ BaseTrait `property:",squash"`
+ // A list of configuration pointing to configmap/secret. Syntax: [configmap|secret]:name[key], where name represents the resource name and key optionally represents the resource key to be filtered
+ Configs []string `property:"configs" json:"configs,omitempty"`
+ // A list of resources pointing to configmap/secret. Syntax: [configmap|secret]:name[/key][@path], where name represents the resource name, key optionally represents the resource key to be filtered and path represents the destination path
+ Resources []string `property:"resources" json:"resources,omitempty"`
+ // A list of Persistent Volume Claims to be mounted. Syntax: [pvcname:/container/path]
+ Volumes []string `property:"volumes" json:"volumes,omitempty"`
+}
+
+func newMountTrait() Trait {
+ return &mountTrait{
+ // Must follow immediately the container trait
+ BaseTrait: NewBaseTrait("mount", 1610),
+ }
+}
+
+func (t *mountTrait) Configure(e *Environment) (bool, error) {
+ if IsFalse(t.Enabled) {
+ return false, nil
+ }
+
+ if !e.IntegrationInPhase(v1.IntegrationPhaseInitialization) && !e.IntegrationInRunningPhases() {
+ return false, nil
+ }
+
+ // Validate resources and pvcs
+ for _, c := range t.Configs {
+ if !strings.HasPrefix(c, "configmap:") && !strings.HasPrefix(c, "secret:") {
+ return false, fmt.Errorf("unsupported config %s, must be a configmap or secret resource", c)
+ }
+ }
+ for _, r := range t.Resources {
+ if !strings.HasPrefix(r, "configmap:") && !strings.HasPrefix(r, "secret:") {
+ return false, fmt.Errorf("unsupported resource %s, must be a configmap or secret resource", r)
+ }
+ }
+
+ return true, nil
+}
+
+func (t *mountTrait) Apply(e *Environment) error {
+ if e.IntegrationInPhase(v1.IntegrationPhaseInitialization) {
Review comment:
Could the `Configure` method return `false` then during the `IntegrationPhaseInitialization` phase?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] astefanutti edited a comment on pull request #2771: feat(cmd/run): autogenerated configmap for resource/config local files
Posted by GitBox <gi...@apache.org>.
astefanutti edited a comment on pull request #2771:
URL: https://github.com/apache/camel-k/pull/2771#issuecomment-998660790
> Thanks a lot for the review @astefanutti , it's a long one, I really appreciate your help with this.
>
> > My understanding is that the implementation assumes the "local file" use case is different than the "standard" ConfigMap use case. It's possible you've already tried to explain the difference, though could you please clarify the rational behind that assumption. It seems that the file prefix / reference gets stored in the container trait configuration, and even if it's weakly linked to the local file, and resolved to the generated ConfigMap, that seems to break the "standalone" configuration principle
>
> The idea we discussed was to go in the direction to [remove any arbitrary file content from the integration](https://github.com/apache/camel-k/issues/2320#issuecomment-922718785). With this PR what we do is to automatically help the user convert a local file into a configmap, and then attach to the Integration. In my opinion what we're doing is to remove the support for local file, although we help the user to convert it in the supported storage via `kamel` CLI to simplify his life. If the user is not able to run an Integration via `kamel run`, in fact, the feature cannot be provided.
That is my background too. My question was more, why does the `file` prefix leaks beyond the CLI, and the generated ConfigMap seems to be treated differently than the other "standard" ones?
> > The generated ConfigMap is owned by the Integration, which makes the former gets garbage collected when the former is deleted. However, how is it garbage collected when the Integration is updated with the file resource removed, for example with a new kamel run invocation, without the resource?
>
> Good point, I did not consider this situation. The configmap is updated when there is a file change and `--sync/--dev` enabled, but I did not check what happens when the file is removed at all. I'll run some test and come back to this point.
>
> > What's the rational, from the functional standpoint, to use the container trait to host the configuration resources? It's already responsible for the compute resources, the container image, the service, and the (deprecated) health probes configuration.
>
> The container trait is already in charge to convert the configmaps/secret/... into mounted volumes ([we discussed it shortly as well](https://github.com/apache/camel-k/pull/2635#issuecomment-922720383)), so it felt the most appropriate trait to use. Otherwise we need somehow to move that logic elsewhere or duplicating it, which I don't think is a good idea.
I'm trying to reason first from the end-user standpoint. The CLI hides it, still it's part of the API. I remember we had the discussion in https://github.com/apache/camel-k/pull/2635, and I was wondering whether that was the right moment to find a more "functionally oriented" trait to capture this concern, and disentangle the _container_ trait.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] squakez commented on pull request #2771: feat(cmd/run): autogenerated configmap for resource/config local files
Posted by GitBox <gi...@apache.org>.
squakez commented on pull request #2771:
URL: https://github.com/apache/camel-k/pull/2771#issuecomment-999488606
> > Thanks a lot for the review @astefanutti , it's a long one, I really appreciate your help with this.
> > > My understanding is that the implementation assumes the "local file" use case is different than the "standard" ConfigMap use case. It's possible you've already tried to explain the difference, though could you please clarify the rational behind that assumption. It seems that the file prefix / reference gets stored in the container trait configuration, and even if it's weakly linked to the local file, and resolved to the generated ConfigMap, that seems to break the "standalone" configuration principle
> >
> >
> > The idea we discussed was to go in the direction to [remove any arbitrary file content from the integration](https://github.com/apache/camel-k/issues/2320#issuecomment-922718785). With this PR what we do is to automatically help the user convert a local file into a configmap, and then attach to the Integration. In my opinion what we're doing is to remove the support for local file, although we help the user to convert it in the supported storage via `kamel` CLI to simplify his life. If the user is not able to run an Integration via `kamel run`, in fact, the feature cannot be provided.
>
> That is my background too. My question was more, why does the `file` prefix beyond the CLI, and the generated ConfigMap seems to be treated differently than the other "standard" ones?
If I understood correctly, the question is about generated Configmap difference with user provided Configmap. The only difference is about the lifecycle of the Configmap itself, as my idea is to have it living together and exclusively within the Integration lifecycle. This is mimicking the previous behavior, when a Configmap was generated beside the Integration. The difference is that the resource is not directly stored in the Integration and the Configmap generated by the CLI (as it must access the file content). Another possibility would be to autogenerate the Configmap but let the lifecycle (update/delete) up to the user. However, I think it completely overlaps what we already provide with the normal configmap, just saving the manual step to create the configmap from the file resource manually.
Hope it answer your question, I'm not entirely sure we're talking about the same thing here :)
>
> > > The generated ConfigMap is owned by the Integration, which makes the former gets garbage collected when the former is deleted. However, how is it garbage collected when the Integration is updated with the file resource removed, for example with a new kamel run invocation, without the resource?
> >
> >
> > Good point, I did not consider this situation. The configmap is updated when there is a file change and `--sync/--dev` enabled, but I did not check what happens when the file is removed at all. I'll run some test and come back to this point.
I made some test and with this implementation, the generated Configmap is kept until the Integration is living. We can provide an additional loop to reconcile the generated Configmap not needed right after the binding step. Or we can keep it the way it is, if we consider the patter to create an Integration with a file and later update the same integration without a file as a rare one. I don't have a strong opinion on this part.
> > > What's the rational, from the functional standpoint, to use the container trait to host the configuration resources? It's already responsible for the compute resources, the container image, the service, and the (deprecated) health probes configuration.
> >
> >
> > The container trait is already in charge to convert the configmaps/secret/... into mounted volumes ([we discussed it shortly as well](https://github.com/apache/camel-k/pull/2635#issuecomment-922720383)), so it felt the most appropriate trait to use. Otherwise we need somehow to move that logic elsewhere or duplicating it, which I don't think is a good idea.
>
> I'm trying to reason first from the end-user standpoint. The CLI hides it, still it's part of the API. I remember we had the discussion in #2635, and I was wondering whether that was the right moment to find a more "functionally oriented" trait to capture this concern, and disentangle the _container_ trait.
The idea to move the _volumes_ logic into a separate trait could be interesting. However, we need to attach those volumes into the `corev1.Container`. If we create a new trait, then it will be very dependent on the container trait, requiring to be executed after it, and it will need to get the `corev1.Container` spec. After all, I think it is responsibility of the container trait to manage its own volumes.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] astefanutti commented on pull request #2771: feat(cmd/run): autogenerated configmap for resource/config local files
Posted by GitBox <gi...@apache.org>.
astefanutti commented on pull request #2771:
URL: https://github.com/apache/camel-k/pull/2771#issuecomment-978957940
For the versioning, I think that it'd be acceptable to apply the current strategy, that is marking the field(s) as deprecated, let the users do the migration, and have the fields removed in later releases. Versioning CRDs is quite involved, and we would possibly have to implement a conversion Webhook, which comes at an extra operational cost.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] astefanutti commented on pull request #2771: feat(cmd/run): autogenerated configmap for resource/config local files
Posted by GitBox <gi...@apache.org>.
astefanutti commented on pull request #2771:
URL: https://github.com/apache/camel-k/pull/2771#issuecomment-998660790
> Thanks a lot for the review @astefanutti , it's a long one, I really appreciate your help with this.
>
> > My understanding is that the implementation assumes the "local file" use case is different than the "standard" ConfigMap use case. It's possible you've already tried to explain the difference, though could you please clarify the rational behind that assumption. It seems that the file prefix / reference gets stored in the container trait configuration, and even if it's weakly linked to the local file, and resolved to the generated ConfigMap, that seems to break the "standalone" configuration principle
>
> The idea we discussed was to go in the direction to [remove any arbitrary file content from the integration](https://github.com/apache/camel-k/issues/2320#issuecomment-922718785). With this PR what we do is to automatically help the user convert a local file into a configmap, and then attach to the Integration. In my opinion what we're doing is to remove the support for local file, although we help the user to convert it in the supported storage via `kamel` CLI to simplify his life. If the user is not able to run an Integration via `kamel run`, in fact, the feature cannot be provided.
That is my background too. My question was more, why does the `file` prefix beyond the CLI, and the generated ConfigMap seems to be treated differently than the other "standard" ones?
> > The generated ConfigMap is owned by the Integration, which makes the former gets garbage collected when the former is deleted. However, how is it garbage collected when the Integration is updated with the file resource removed, for example with a new kamel run invocation, without the resource?
>
> Good point, I did not consider this situation. The configmap is updated when there is a file change and `--sync/--dev` enabled, but I did not check what happens when the file is removed at all. I'll run some test and come back to this point.
>
> > What's the rational, from the functional standpoint, to use the container trait to host the configuration resources? It's already responsible for the compute resources, the container image, the service, and the (deprecated) health probes configuration.
>
> The container trait is already in charge to convert the configmaps/secret/... into mounted volumes ([we discussed it shortly as well](https://github.com/apache/camel-k/pull/2635#issuecomment-922720383)), so it felt the most appropriate trait to use. Otherwise we need somehow to move that logic elsewhere or duplicating it, which I don't think is a good idea.
I'm trying to reason first from the end-user standpoint. The CLI hides it, still it's part of the API. I remember we had the discussion in https://github.com/apache/camel-k/pull/2635, and I was wondering whether that was the right moment to find a more "functionally oriented" trait to capture this concern, and disentangle the _container_ trait.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] squakez commented on pull request #2771: feat(cmd/run): autogenerated configmap for resource/config local files
Posted by GitBox <gi...@apache.org>.
squakez commented on pull request #2771:
URL: https://github.com/apache/camel-k/pull/2771#issuecomment-996749014
@astefanutti if you can find some time to review it, I'd appreciate it. The first part is the same you had a look some time ago, the rest of the commits implemented the different things planned originally. Thanks!
--
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.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] lburgazzoli edited a comment on pull request #2771: feat(cmd/run): autogenerated configmap for resource/config local files
Posted by GitBox <gi...@apache.org>.
lburgazzoli edited a comment on pull request #2771:
URL: https://github.com/apache/camel-k/pull/2771#issuecomment-976550465
I'm not deep into the code at the moment to do a review but I'm +1 on the goal.
What we have to think about is the backward compatibility so if we remove the ResourceSpec, we may need to bump APIs to v2 ?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] squakez commented on pull request #2771: feat(cmd/run): autogenerated configmap for resource/config local files
Posted by GitBox <gi...@apache.org>.
squakez commented on pull request #2771:
URL: https://github.com/apache/camel-k/pull/2771#issuecomment-1007333096
@astefanutti any last look before I can merge?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] squakez merged pull request #2771: feat(cmd/run): autogenerated configmap for resource/config local files
Posted by GitBox <gi...@apache.org>.
squakez merged pull request #2771:
URL: https://github.com/apache/camel-k/pull/2771
--
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.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] squakez edited a comment on pull request #2771: feat(cmd/run): autogenerated configmap for resource/config local files
Posted by GitBox <gi...@apache.org>.
squakez edited a comment on pull request #2771:
URL: https://github.com/apache/camel-k/pull/2771#issuecomment-999488606
> > Thanks a lot for the review @astefanutti , it's a long one, I really appreciate your help with this.
> > > My understanding is that the implementation assumes the "local file" use case is different than the "standard" ConfigMap use case. It's possible you've already tried to explain the difference, though could you please clarify the rational behind that assumption. It seems that the file prefix / reference gets stored in the container trait configuration, and even if it's weakly linked to the local file, and resolved to the generated ConfigMap, that seems to break the "standalone" configuration principle
> >
> >
> > The idea we discussed was to go in the direction to [remove any arbitrary file content from the integration](https://github.com/apache/camel-k/issues/2320#issuecomment-922718785). With this PR what we do is to automatically help the user convert a local file into a configmap, and then attach to the Integration. In my opinion what we're doing is to remove the support for local file, although we help the user to convert it in the supported storage via `kamel` CLI to simplify his life. If the user is not able to run an Integration via `kamel run`, in fact, the feature cannot be provided.
>
> That is my background too. My question was more, why does the `file` prefix leaks beyond the CLI, and the generated ConfigMap seems to be treated differently than the other "standard" ones?
If I understood correctly, the question is about generated Configmap difference with user provided Configmap. The only difference is about the lifecycle of the Configmap itself, as my idea is to have it living together and exclusively within the Integration lifecycle. This is mimicking the previous behavior, when a Configmap was generated beside the Integration. The difference is that the resource is not directly stored in the Integration and the Configmap is generated by the CLI (as it must access the file content). Another possibility would be to autogenerate the Configmap but let the lifecycle (update/delete) up to the user. However, I think it completely overlaps what we already provide with the normal configmap, just saving the manual step to create the configmap from the file resource.
Hope it answer your question, I'm not entirely sure we're talking about the same thing here :)
>
> > > The generated ConfigMap is owned by the Integration, which makes the former gets garbage collected when the former is deleted. However, how is it garbage collected when the Integration is updated with the file resource removed, for example with a new kamel run invocation, without the resource?
> >
> >
> > Good point, I did not consider this situation. The configmap is updated when there is a file change and `--sync/--dev` enabled, but I did not check what happens when the file is removed at all. I'll run some test and come back to this point.
I made some test and with this implementation, the generated Configmap is kept until the Integration is living. We can provide an additional loop to reconcile the generated Configmap not needed right after the binding step. Or we can keep it the way it is, if we consider the pattern to create an Integration with a file and later update the same integration without a file as a rare one. I don't have a strong opinion on this part.
> > > What's the rational, from the functional standpoint, to use the container trait to host the configuration resources? It's already responsible for the compute resources, the container image, the service, and the (deprecated) health probes configuration.
> >
> >
> > The container trait is already in charge to convert the configmaps/secret/... into mounted volumes ([we discussed it shortly as well](https://github.com/apache/camel-k/pull/2635#issuecomment-922720383)), so it felt the most appropriate trait to use. Otherwise we need somehow to move that logic elsewhere or duplicating it, which I don't think is a good idea.
>
> I'm trying to reason first from the end-user standpoint. The CLI hides it, still it's part of the API. I remember we had the discussion in #2635, and I was wondering whether that was the right moment to find a more "functionally oriented" trait to capture this concern, and disentangle the _container_ trait.
The idea to move the _volumes_ logic into a separate trait could be interesting. However, we need to attach those volumes into the `corev1.Container`. If we create a new trait, then it will be very dependent on the container trait, requiring to be executed after it, and it will need to get the `corev1.Container` spec. After all, I think it is responsibility of the container trait to manage its own volumes.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] astefanutti commented on a change in pull request #2771: feat(cmd/run): autogenerated configmap for resource/config local files
Posted by GitBox <gi...@apache.org>.
astefanutti commented on a change in pull request #2771:
URL: https://github.com/apache/camel-k/pull/2771#discussion_r777994177
##########
File path: pkg/util/kubernetes/factory.go
##########
@@ -32,6 +33,12 @@ var (
validResourceRequirementsRegexp = regexp.MustCompile(`^(requests|limits)\.(memory|cpu)=([\w\.]+)$`)
)
+// ConfigMapAutogenLabel --
+const ConfigMapAutogenLabel = "camel.apache.org/autogenerated"
Review comment:
May I suggest `camel.apache.org/generated` instead?
##########
File path: pkg/trait/mount.go
##########
@@ -0,0 +1,193 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package trait
+
+import (
+ "fmt"
+ "strings"
+
+ appsv1 "k8s.io/api/apps/v1"
+ "k8s.io/api/batch/v1beta1"
+ corev1 "k8s.io/api/core/v1"
+
+ serving "knative.dev/serving/pkg/apis/serving/v1"
+
+ v1 "github.com/apache/camel-k/pkg/apis/camel/v1"
+ "github.com/apache/camel-k/pkg/util/kubernetes"
+ utilResource "github.com/apache/camel-k/pkg/util/resource"
+)
+
+// The Mount trait can be used to configure volumes mounted on the Integration Pods.
+//
+// +camel-k:trait=mount
+// nolint: tagliatelle
+type mountTrait struct {
+ BaseTrait `property:",squash"`
+ // A list of configuration (only text content) pointing to configmap/secret. Syntax: [configmap|secret]:name[key], where name represents the resource name and key optionally represents the resource key to be filtered
+ Configs []string `property:"configs" json:"configs,omitempty"`
+ // A list of resources (text or binary content) pointing to configmap/secret. Syntax: [configmap|secret]:name[/key][@path], where name represents the resource name, key optionally represents the resource key to be filtered and path represents the destination path
+ Resources []string `property:"resources" json:"resources,omitempty"`
+ // A list of Persistent Volume Claims to be mounted. Syntax: [pvcname:/container/path]
+ Volumes []string `property:"volumes" json:"volumes,omitempty"`
+}
+
+func newMountTrait() Trait {
+ return &mountTrait{
+ // Must follow immediately the container trait
+ BaseTrait: NewBaseTrait("mount", 1610),
+ }
+}
+
+func (t *mountTrait) Configure(e *Environment) (bool, error) {
+ if IsFalse(t.Enabled) {
+ return false, nil
+ }
+
+ if e.IntegrationInPhase(v1.IntegrationPhaseInitialization) {
+ return false, nil
+ }
+
+ // Validate resources and pvcs
+ for _, c := range t.Configs {
+ if !strings.HasPrefix(c, "configmap:") && !strings.HasPrefix(c, "secret:") {
+ return false, fmt.Errorf("unsupported config %s, must be a configmap or secret resource", c)
+ }
+ }
+ for _, r := range t.Resources {
+ if !strings.HasPrefix(r, "configmap:") && !strings.HasPrefix(r, "secret:") {
+ return false, fmt.Errorf("unsupported resource %s, must be a configmap or secret resource", r)
+ }
+ }
+
+ return true, nil
+}
+
+func (t *mountTrait) Apply(e *Environment) error {
+ if e.IntegrationInPhase(v1.IntegrationPhaseInitialization) {
+ return nil
+ }
+
+ container := e.GetIntegrationContainer()
+ if container == nil {
+ return fmt.Errorf("unable to find integration container: %s", e.Integration.Name)
+ }
+
+ var volumes *[]corev1.Volume
+ visited := false
+
+ // Deployment
+ if err := e.Resources.VisitDeploymentE(func(deployment *appsv1.Deployment) error {
+ volumes = &deployment.Spec.Template.Spec.Volumes
+ visited = true
+ return nil
+ }); err != nil {
+ return err
+ }
+
+ // Knative Service
+ if err := e.Resources.VisitKnativeServiceE(func(service *serving.Service) error {
+ volumes = &service.Spec.ConfigurationSpec.Template.Spec.Volumes
+ visited = true
+ return nil
+ }); err != nil {
+ return err
+ }
+
+ // CronJob
+ if err := e.Resources.VisitCronJobE(func(cron *v1beta1.CronJob) error {
+ volumes = &cron.Spec.JobTemplate.Spec.Template.Spec.Volumes
+ visited = true
+ return nil
+ }); err != nil {
+ return err
+ }
+
+ if visited {
+ // Volumes declared in the Integration resources
+ e.configureVolumesAndMounts(volumes, &container.VolumeMounts)
+ // Volumes declared in the trait config/resource options
+ err := t.configureVolumesAndMounts(e, volumes, &container.VolumeMounts)
+ if err != nil {
+ return err
+ }
+ }
+
+ return nil
+}
+
+func (t *mountTrait) configureVolumesAndMounts(e *Environment, vols *[]corev1.Volume, mnts *[]corev1.VolumeMount) error {
+ for _, c := range t.Configs {
+ if conf, parseErr := utilResource.ParseConfig(c); parseErr == nil {
+ t.attachResource(e, conf)
+ t.mountResource(vols, mnts, conf)
+ } else {
+ return parseErr
+ }
+ }
+ for _, r := range t.Resources {
+ if res, parseErr := utilResource.ParseResource(r); parseErr == nil {
+ t.attachResource(e, res)
+ t.mountResource(vols, mnts, res)
+ } else {
+ return parseErr
+ }
+ }
+ for _, v := range t.Volumes {
+ if vol, parseErr := utilResource.ParseVolume(v); parseErr == nil {
+ t.mountResource(vols, mnts, vol)
+ } else {
+ return parseErr
+ }
+ }
+
+ return nil
+}
+
+// attachResource is in charge to filter the autogenerated configmap and attach to the Integration resources.
+// The owner trait will be in charge to bind it accordingly.
+func (t *mountTrait) attachResource(e *Environment, conf *utilResource.Config) {
+ if conf.StorageType() == utilResource.StorageTypeConfigmap {
+ // verify if it was autogenerated
+ cm := kubernetes.LookupConfigmap(e.Ctx, e.Client, e.Integration.Namespace, conf.Name())
+ if cm != nil && cm.ObjectMeta.Labels[kubernetes.ConfigMapAutogenLabel] == "true" {
+ // we must remove the managed fields as owner trait later will complain otherwise
+ cm.ManagedFields = nil
Review comment:
I'd suggest to try adding an empty ConfigMap object with the same namespace and name instead. That way we make sure the original ConfigMap is left untouched, the other traits like the _owner_ one, add their bits, then the _deployer_ trait relies on server-side apply to merge everything. It should avoid that `cm.ManagedFields = nil`.
##########
File path: pkg/trait/mount.go
##########
@@ -0,0 +1,177 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package trait
+
+import (
+ "fmt"
+ "strings"
+
+ appsv1 "k8s.io/api/apps/v1"
+ "k8s.io/api/batch/v1beta1"
+ corev1 "k8s.io/api/core/v1"
+
+ serving "knative.dev/serving/pkg/apis/serving/v1"
+
+ v1 "github.com/apache/camel-k/pkg/apis/camel/v1"
+ "github.com/apache/camel-k/pkg/util/kubernetes"
+ utilResource "github.com/apache/camel-k/pkg/util/resource"
+)
+
+// The Mount trait can be used to configure volumes mounted on the Integration Pod.
+//
+// +camel-k:trait=mount
+// nolint: tagliatelle
+type mountTrait struct {
+ BaseTrait `property:",squash"`
+ // A list of configuration pointing to configmap/secret. Syntax: [configmap|secret]:name[key], where name represents the resource name and key optionally represents the resource key to be filtered
+ Configs []string `property:"configs" json:"configs,omitempty"`
Review comment:
The comment now documents what kind of content can be provided. I could still imagine an external user asking what's the difference between configs and resources functionally. May I suggest to add that configs are added to the classpath, and that they are typically used to configuration Camel.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] squakez commented on pull request #2771: feat(cmd/run): autogenerated configmap for resource/config local files
Posted by GitBox <gi...@apache.org>.
squakez commented on pull request #2771:
URL: https://github.com/apache/camel-k/pull/2771#issuecomment-1004700084
Very clever suggestion @astefanutti. I like how it looks like now. Please see 1b405ee which has captured the changes you proposed. Both `mount` and `openapi` will look for autogenerated configmaps and let the `owner` and `gc` performs their responsibility.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] lburgazzoli commented on pull request #2771: feat(cmd/run): autogenerated configmap for resource/config local files
Posted by GitBox <gi...@apache.org>.
lburgazzoli commented on pull request #2771:
URL: https://github.com/apache/camel-k/pull/2771#issuecomment-976550465
I'm not deep into the code at the moment to do a review but I'm +1 on the goal
--
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.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] squakez commented on pull request #2771: feat(cmd/run): autogenerated configmap for resource/config local files
Posted by GitBox <gi...@apache.org>.
squakez commented on pull request #2771:
URL: https://github.com/apache/camel-k/pull/2771#issuecomment-978997002
Thanks for the review @astefanutti, those are very good points to discuss.
> A couple of points:
>
> * Would the ultimate place to reference these ConfigMaps be in a new _configuration_ trait, as discussed in #2320, maybe as a second iteration to this?
That is partially correct. The idea is to move configmap/secret managment there as soon as this part would be completed. So, once autogenerated, the configmaps will be managed as any user provided configmap into the `configuration` trait. However, the generation part must be done in the CLI, because the trait is executed by the operator and won't be able to access the local file content.
>
> * Should the operator watch these owned ConfigMaps for changes, and reconcile the parent Integration?
>
> * If yes, as a sub-case, the Integration phase could be set to `Error` if a referenced ConfigMap got deleted by mistake.
> * It relates to #1235 and #2106 possibly
>
I don't think this would apply to autogenerated configmaps. In my view, these kind of configmaps are just a support to the local file content. I've worked to support `sync` option and have an Integration redeploy when we detect a change in the local file, that will translate in a **new** configmap. However, in a production environment, I expect the user to provide "normal" configmaps if there is the possibility to change the underlying content (and in that case still apply the issues reported above).
> * Are the generated ConfigMaps really immutable?
They must be, IMO. The name is generated from a SHA based on the file content and nobody should alter them as they are thought as a representation of the local file.
>
> * Would that make sense to go the same direction for the `SourceSpec` field?
Not sure if it makes sense. I think that the `SourceSpec` is a core part of the Integration, and we must allow anybody that want to provide an Integration with `kubectl` to do that without adding additional steps (ie, creating a configmap, then setting it in the Integration). A resource, instead, is already something complementary to the Integration. If we go in this direction, in fact, the user that creates directly Integration won't be allowed any longer to create a `ResourceSpec` with content and will need to create a configmap on his own.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] squakez commented on pull request #2771: feat(cmd/run): autogenerated configmap for resource/config local files
Posted by GitBox <gi...@apache.org>.
squakez commented on pull request #2771:
URL: https://github.com/apache/camel-k/pull/2771#issuecomment-1007333096
@astefanutti any last look before I can merge?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] squakez commented on pull request #2771: feat(cmd/run): autogenerated configmap for resource/config local files
Posted by GitBox <gi...@apache.org>.
squakez commented on pull request #2771:
URL: https://github.com/apache/camel-k/pull/2771#issuecomment-976583410
> I'm not deep into the code at the moment to do a review but I'm +1 on the goal. What we have to think about is the backward compatibility so if we remove the ResourceSpec, we may need to bump APIs to v2 ?
No problem, more than the code, the goal of the draft PR is to discuss about the feature. About bumping API version, I actually don't know which would be the recommended procedures to follow.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] squakez commented on pull request #2771: feat(cmd/run): autogenerated configmap for resource/config local files
Posted by GitBox <gi...@apache.org>.
squakez commented on pull request #2771:
URL: https://github.com/apache/camel-k/pull/2771#issuecomment-998647128
Thanks a lot for the review @astefanutti , it's a long one, I really appreciate your help with this.
> My understanding is that the implementation assumes the "local file" use case is different than the "standard" ConfigMap use case. It's possible you've already tried to explain the difference, though could you please clarify the rational behind that assumption. It seems that the file prefix / reference gets stored in the container trait configuration, and even if it's weakly linked to the local file, and resolved to the generated ConfigMap, that seems to break the "standalone" configuration principle
The idea we discussed was to go in the direction to [remove any arbitrary file content from the integration](https://github.com/apache/camel-k/issues/2320#issuecomment-922718785). With this PR what we do is to automatically help the user convert a local file into a configmap, and then attach to the Integration. In my opinion what we're doing is to remove the support for local file, although we help the user to convert it in the supported storage via `kamel` CLI to simplify his life. If the user is not able to run an Integration via `kamel run`, in fact, the feature cannot be provided.
> The generated ConfigMap is owned by the Integration, which makes the former gets garbage collected when the former is deleted. However, how is it garbage collected when the Integration is updated with the file resource removed, for example with a new kamel run invocation, without the resource?
Good point, I did not consider this situation. The configmap is updated when there is a file change and `--sync/--dev` enabled, but I did not check what happens when the file is removed at all. I'll run some test and come back to this point.
> What's the rational, from the functional standpoint, to use the container trait to host the configuration resources? It's already responsible for the compute resources, the container image, the service, and the (deprecated) health probes configuration.
The container trait is already in charge to convert the configmaps/secret/... into mounted volumes ([we discussed it shortly as well](https://github.com/apache/camel-k/pull/2635#issuecomment-922720383)), so it felt the most appropriate trait to use. Otherwise we need somehow to move that logic elsewhere or duplicating it, which I don't think is a good idea.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] squakez commented on pull request #2771: feat(cmd/run): autogenerated configmap for resource/config local files
Posted by GitBox <gi...@apache.org>.
squakez commented on pull request #2771:
URL: https://github.com/apache/camel-k/pull/2771#issuecomment-999536039
> > > > Thanks a lot for the review @astefanutti , it's a long one, I really appreciate your help with this.
> > > > > My understanding is that the implementation assumes the "local file" use case is different than the "standard" ConfigMap use case. It's possible you've already tried to explain the difference, though could you please clarify the rational behind that assumption. It seems that the file prefix / reference gets stored in the container trait configuration, and even if it's weakly linked to the local file, and resolved to the generated ConfigMap, that seems to break the "standalone" configuration principle
> > > >
> > > >
> > > > The idea we discussed was to go in the direction to [remove any arbitrary file content from the integration](https://github.com/apache/camel-k/issues/2320#issuecomment-922718785). With this PR what we do is to automatically help the user convert a local file into a configmap, and then attach to the Integration. In my opinion what we're doing is to remove the support for local file, although we help the user to convert it in the supported storage via `kamel` CLI to simplify his life. If the user is not able to run an Integration via `kamel run`, in fact, the feature cannot be provided.
> > >
> > >
> > > That is my background too. My question was more, why does the `file` prefix beyond the CLI, and the generated ConfigMap seems to be treated differently than the other "standard" ones?
> >
> >
> > If I understood correctly, the question is about generated Configmap difference with user provided Configmap. The only difference is about the lifecycle of the Configmap itself, as my idea is to have it living together and exclusively within the Integration lifecycle. This is mimicking the previous behavior, when a Configmap was generated beside the Integration. The difference is that the resource is not directly stored in the Integration and the Configmap is generated by the CLI (as it must access the file content). Another possibility would be to autogenerate the Configmap but let the lifecycle (update/delete) up to the user. However, I think it completely overlaps what we already provide with the normal configmap, just saving the manual step to create the configmap from the file resource.
> > Hope it answer your question, I'm not entirely sure we're talking about the same thing here :)
>
> I think we are talking about the same thing :) But that is general description, that I still fail to reconcile with the actual implementation :) Maybe this could be cleared if you could help me understand:
>
> * Why the file information leaks into the _container_ trait configuration, following the comment ` Syntax: [configmap|secret|file]:name[key], where name represents the local file path or the configmap/secret` and my interpretation of the current implementation?
>
Ah, this is a mistake. I copied the description from the run command. In the trait we cannot have a `file` but only `configmap|secret`. The run is in charge to convert a file into the related autogenerated configmap, ie , `-t container.resource=configmap:cm-xyz`.
> * Also why the generated ConfigMap name is hashed based on local information? Could we mimic what `kubectl create configmap` does?
>
Originally I used the file name as a configmap name. But that leads to 2 problems:
1. We are not able to provide the same file name in the namespace, ie, `kamel run -n i1 -r file:test.txt` and `kamel run -n i2 -r file:test.txt` as the second would reuse the content of the previously created configmap
2. We are not able to detect easily a change in the content, as we need to inspect the content of the configmaps when `--sync` is enabled
Using an hash solves both problems.
>
> I'm trying to understand if the consistency of the Integration configuration being created is still satisfied, meaning if user A create an integration with a local file resource, user B should still be able to operate the Integration. I hope that makes sense :)
>
> > > > > The generated ConfigMap is owned by the Integration, which makes the former gets garbage collected when the former is deleted. However, how is it garbage collected when the Integration is updated with the file resource removed, for example with a new kamel run invocation, without the resource?
> > > >
> > > >
> > > > Good point, I did not consider this situation. The configmap is updated when there is a file change and `--sync/--dev` enabled, but I did not check what happens when the file is removed at all. I'll run some test and come back to this point.
> >
> >
> > I made some test and with this implementation, the generated Configmap is kept until the Integration is living. We can provide an additional loop to reconcile the generated Configmap not needed right after the binding step. Or we can keep it the way it is, if we consider the pattern to create an Integration with a file and later update the same integration without a file as a rare one. I don't have a strong opinion on this part.
>
> What I'm trying to determine here, aren't we about to duplicate the garbage collection that we already have for resources generated by the operator. I get the particularity of the "local file" use case, but would there be a way to make it fit into the existing garbage collection mechanism.
>
Yes, reason why I'd keep the generated content until the Integration lives.
> > > > > What's the rational, from the functional standpoint, to use the container trait to host the configuration resources? It's already responsible for the compute resources, the container image, the service, and the (deprecated) health probes configuration.
> > > >
> > > >
> > > > The container trait is already in charge to convert the configmaps/secret/... into mounted volumes ([we discussed it shortly as well](https://github.com/apache/camel-k/pull/2635#issuecomment-922720383)), so it felt the most appropriate trait to use. Otherwise we need somehow to move that logic elsewhere or duplicating it, which I don't think is a good idea.
> > >
> > >
> > > I'm trying to reason first from the end-user standpoint. The CLI hides it, still it's part of the API. I remember we had the discussion in #2635, and I was wondering whether that was the right moment to find a more "functionally oriented" trait to capture this concern, and disentangle the _container_ trait.
> >
> >
> > The idea to move the _volumes_ logic into a separate trait could be interesting. However, we need to attach those volumes into the `corev1.Container`. If we create a new trait, then it will be very dependent on the container trait, requiring to be executed after it, and it will need to get the `corev1.Container` spec. After all, I think it is responsibility of the container trait to manage its own volumes.
>
> Yes, that is also my understanding of the technical details. I'm trying to reason at the functional level, from the end-user standpoint.
Definitely, the usage of `container` is driven by the fact that, technically speaking, the volumes belong to the Container. As we had the option only in the `kamel run`, so far, we did not expose that kind of information. If we move into a trait it becomes public, so, we may think to move into maybe a `volume` trait. Or maybe a `resource` trait.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] astefanutti commented on a change in pull request #2771: feat(cmd/run): autogenerated configmap for resource/config local files
Posted by GitBox <gi...@apache.org>.
astefanutti commented on a change in pull request #2771:
URL: https://github.com/apache/camel-k/pull/2771#discussion_r778378721
##########
File path: pkg/trait/mount.go
##########
@@ -0,0 +1,199 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package trait
+
+import (
+ "fmt"
+ "strings"
+
+ appsv1 "k8s.io/api/apps/v1"
+ "k8s.io/api/batch/v1beta1"
+ corev1 "k8s.io/api/core/v1"
+
+ serving "knative.dev/serving/pkg/apis/serving/v1"
+
+ v1 "github.com/apache/camel-k/pkg/apis/camel/v1"
+ "github.com/apache/camel-k/pkg/util/kubernetes"
+ utilResource "github.com/apache/camel-k/pkg/util/resource"
+)
+
+// The Mount trait can be used to configure volumes mounted on the Integration Pods.
+//
+// +camel-k:trait=mount
+// nolint: tagliatelle
+type mountTrait struct {
+ BaseTrait `property:",squash"`
+ // A list of configuration pointing to configmap/secret.
+ // The configuration are expected to be UTF-8 resources as they are processed by runtime Camel Context and tried to be parsed as property files.
+ // They are also made available on the classpath in order to ease their usage directly from the Route.
+ // Syntax: [configmap|secret]:name[key], where name represents the resource name and key optionally represents the resource key to be filtered
+ Configs []string `property:"configs" json:"configs,omitempty"`
+ // A list of resources (text or binary content) pointing to configmap/secret.
+ // The resources are expected to be any resource type (text or binary content).
+ // The destination path can be either a default location or any path specified by the user.
+ // Syntax: [configmap|secret]:name[/key][@path], where name represents the resource name, key optionally represents the resource key to be filtered and path represents the destination path
+ Resources []string `property:"resources" json:"resources,omitempty"`
+ // A list of Persistent Volume Claims to be mounted. Syntax: [pvcname:/container/path]
+ Volumes []string `property:"volumes" json:"volumes,omitempty"`
+}
+
+func newMountTrait() Trait {
+ return &mountTrait{
+ // Must follow immediately the container trait
+ BaseTrait: NewBaseTrait("mount", 1610),
+ }
+}
+
+func (t *mountTrait) Configure(e *Environment) (bool, error) {
+ if IsFalse(t.Enabled) {
+ return false, nil
+ }
+
+ if e.IntegrationInPhase(v1.IntegrationPhaseInitialization) ||
+ (!e.IntegrationInPhase(v1.IntegrationPhaseInitialization) && !e.IntegrationInRunningPhases()) {
+ return false, nil
+ }
+
+ // Validate resources and pvcs
+ for _, c := range t.Configs {
+ if !strings.HasPrefix(c, "configmap:") && !strings.HasPrefix(c, "secret:") {
+ return false, fmt.Errorf("unsupported config %s, must be a configmap or secret resource", c)
+ }
+ }
+ for _, r := range t.Resources {
+ if !strings.HasPrefix(r, "configmap:") && !strings.HasPrefix(r, "secret:") {
+ return false, fmt.Errorf("unsupported resource %s, must be a configmap or secret resource", r)
+ }
+ }
+
+ return true, nil
+}
+
+func (t *mountTrait) Apply(e *Environment) error {
+ if e.IntegrationInPhase(v1.IntegrationPhaseInitialization) {
+ return nil
+ }
+
+ container := e.GetIntegrationContainer()
+ if container == nil {
+ return fmt.Errorf("unable to find integration container: %s", e.Integration.Name)
+ }
+
+ var volumes *[]corev1.Volume
+ visited := false
+
+ // Deployment
+ if err := e.Resources.VisitDeploymentE(func(deployment *appsv1.Deployment) error {
+ volumes = &deployment.Spec.Template.Spec.Volumes
+ visited = true
+ return nil
+ }); err != nil {
+ return err
+ }
+
+ // Knative Service
+ if err := e.Resources.VisitKnativeServiceE(func(service *serving.Service) error {
+ volumes = &service.Spec.ConfigurationSpec.Template.Spec.Volumes
+ visited = true
+ return nil
+ }); err != nil {
+ return err
+ }
+
+ // CronJob
+ if err := e.Resources.VisitCronJobE(func(cron *v1beta1.CronJob) error {
+ volumes = &cron.Spec.JobTemplate.Spec.Template.Spec.Volumes
+ visited = true
+ return nil
+ }); err != nil {
+ return err
+ }
+
+ if visited {
+ // Volumes declared in the Integration resources
+ e.configureVolumesAndMounts(volumes, &container.VolumeMounts)
+ // Volumes declared in the trait config/resource options
+ err := t.configureVolumesAndMounts(e, volumes, &container.VolumeMounts)
+ if err != nil {
+ return err
+ }
+ }
+
+ return nil
+}
+
+func (t *mountTrait) configureVolumesAndMounts(e *Environment, vols *[]corev1.Volume, mnts *[]corev1.VolumeMount) error {
+ for _, c := range t.Configs {
+ if conf, parseErr := utilResource.ParseConfig(c); parseErr == nil {
+ t.attachResource(e, conf)
+ t.mountResource(vols, mnts, conf)
+ } else {
+ return parseErr
+ }
+ }
+ for _, r := range t.Resources {
+ if res, parseErr := utilResource.ParseResource(r); parseErr == nil {
+ t.attachResource(e, res)
+ t.mountResource(vols, mnts, res)
+ } else {
+ return parseErr
+ }
+ }
+ for _, v := range t.Volumes {
+ if vol, parseErr := utilResource.ParseVolume(v); parseErr == nil {
+ t.mountResource(vols, mnts, vol)
+ } else {
+ return parseErr
+ }
+ }
+
+ return nil
+}
+
+// attachResource is in charge to filter the autogenerated configmap and attach to the Integration resources.
+// The owner trait will be in charge to bind it accordingly.
+func (t *mountTrait) attachResource(e *Environment, conf *utilResource.Config) {
+ if conf.StorageType() == utilResource.StorageTypeConfigmap {
+ // verify if it was autogenerated
+ cm := kubernetes.LookupConfigmap(e.Ctx, e.Client, e.Integration.Namespace, conf.Name())
Review comment:
I'm realising those new calls to `kubernetes.LookupConfigmap` will start watching ConfigMap resources, possibly on the entire cluster for the global setup. This should ideally be avoided. One option is to use the _non-caching_ client, or work with unstructured ConfigMap, as caching is disabled for unstructured objects by default.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] astefanutti commented on pull request #2771: feat(cmd/run): autogenerated configmap for resource/config local files
Posted by GitBox <gi...@apache.org>.
astefanutti commented on pull request #2771:
URL: https://github.com/apache/camel-k/pull/2771#issuecomment-1007414198
> @astefanutti any last look before I can merge?
@squakez LGTM 👍🏼 Great work!
--
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.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org