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 2022/04/22 19:53:28 UTC
[GitHub] [camel-k] haanhvu opened a new pull request, #3230: Add a property to Mount trait that creates then mounts a PVC
haanhvu opened a new pull request, #3230:
URL: https://github.com/apache/camel-k/pull/3230
**Release Note**
```release-note
NONE
```
fixes #2994
@squakez please review. If you agree with this implementation, I'll go ahead with the test and doc
--
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 a diff in pull request #3230: Add a property to Mount trait that creates then mounts a PVC
Posted by GitBox <gi...@apache.org>.
squakez commented on code in PR #3230:
URL: https://github.com/apache/camel-k/pull/3230#discussion_r857336833
##########
pkg/trait/mount.go:
##########
@@ -52,6 +52,8 @@ type mountTrait struct {
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"`
+ // A Persistent Volume Claim to be created and mounted. Syntax: pvcname:storageClassName:requestedQuantity:/container/path
+ PVC string `property:"pvc" json:"pvc,omitempty"`
Review Comment:
Probably we should have a slice of PVCs instead
##########
pkg/util/resource/config.go:
##########
@@ -170,6 +172,40 @@ func ParseVolume(item string) (*Config, error) {
}, nil
}
+// CreateAndParseVolume will create and parse a volume and return a Config.
+func CreateAndParseVolume(item string) (*Config, error) {
+ pvcParts := strings.Split(item, ":")
+
+ if len(pvcParts) != 4 {
+ return nil, fmt.Errorf("could not create and mount pvc as %s", item)
+ }
+
+ pvc := corev1.PersistentVolumeClaim{
Review Comment:
I think this object must be passed back to the caller. There you must invoke a function to store the PVC just created in the cluster. As an example, see what we do for the Service trait. We create it and then we apply it on the cluster: https://github.com/apache/camel-k/blob/main/pkg/trait/service.go#L112
--
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 #3230: Add a property to Mount trait that creates then mounts a PVC
Posted by GitBox <gi...@apache.org>.
squakez commented on PR #3230:
URL: https://github.com/apache/camel-k/pull/3230#issuecomment-1113031441
The latest changes have hardly broken the build. Please, have a look.
--
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] haanhvu commented on pull request #3230: Add a property to Mount trait that creates then mounts a PVC
Posted by GitBox <gi...@apache.org>.
haanhvu commented on PR #3230:
URL: https://github.com/apache/camel-k/pull/3230#issuecomment-1113254458
> The latest changes have hardly broken the build. Please, have a look.
Yeah the errors are mostly about go syntax and kubernetes api. I'll take a look shortly.
--
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] haanhvu commented on pull request #3230: Add a property to Mount trait that creates then mounts a PVC
Posted by GitBox <gi...@apache.org>.
haanhvu commented on PR #3230:
URL: https://github.com/apache/camel-k/pull/3230#issuecomment-1125758373
> It looks good now. Please, remind to we should have an example and possibly an e2e test to validate the new functionality before merging.
@squakez yeah I'm trying to create an example.
The new property is recognized. However, the pvc is not created with this error: `User "system:serviceaccount:default:camel-k-operator" cannot patch resource "persistentvolumeclaims" in API group "" at the cluster scope`
Any ideas what's the problem here?
--
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 #3230: Add a property to Mount trait that creates then mounts a PVC
Posted by GitBox <gi...@apache.org>.
squakez commented on PR #3230:
URL: https://github.com/apache/camel-k/pull/3230#issuecomment-1128957475
About 1) I think it would be nice to let the user provide the info, so we might add to the trait parse. As for 2) I think you're missing in the creation of the `PersistentVolumeClaim` struct to define the namespace, so probably it tries to install at cluster scope, for which the namespaced operator has no privileges. Try to amend that and see how it goes.
--
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] github-actions[bot] commented on pull request #3230: Add a property to Mount trait that creates then mounts a PVC
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #3230:
URL: https://github.com/apache/camel-k/pull/3230#issuecomment-1250421560
This PR has been automatically marked as stale due to 90 days of inactivity.
It will be closed if no further activity occurs within 15 days.
If you think that’s incorrect or the issue should never stale, please simply write any comment.
Thanks for your contributions!
--
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] haanhvu commented on pull request #3230: Add a property to Mount trait that creates then mounts a PVC
Posted by GitBox <gi...@apache.org>.
haanhvu commented on PR #3230:
URL: https://github.com/apache/camel-k/pull/3230#issuecomment-1128776648
@squakez when creating the example I found out several things:
1. `accessModes` was missed in the pvc definition. So I added a ReadWriteOnce accessModes to the pvc definition. In fact another option would be let users specify on the accessModes in the trait property. What do you prefer?
2. The pvc couldn't be created due to this error: `Cannot reconcile Integration routes: error executing post actions: error during apply resource: /pvc-example1: persistentvolumeclaims "pvc-example1" is forbidden: User "system:serviceaccount:default:camel-k-operator" cannot patch resource "persistentvolumeclaims" in API group "" at the cluster scope`.
This can be easily fixed with the `kubectl create clusterrolebinding` command. However, is this strange that the operator cannot patch pvc by default? Because from what I see in https://github.com/apache/camel-k/blob/main/config/rbac/operator-role.yaml#L70&L80 the operator has the role to patch pvc
--
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