redhat-developer / odo

odo - Developer-focused CLI for fast & iterative container-based application development on Podman and Kubernetes. Implementation of the open Devfile standard.
https://odo.dev
Apache License 2.0
795 stars 243 forks source link

`odo storage create` seems to have erroneous naming rules #4981

Closed dharmit closed 2 years ago

dharmit commented 3 years ago

/kind bug /area storage

What versions of software are you using?

Operating System: Fedora

Output of odo version: main

How did you run odo exactly?

Actual behavior

$ odo push
 ✗  invalid devfile schema. errors :
- components.0.container.volumeMounts.0.name: Does not match pattern '^[a-z0-9]([-a-z0-9]*[a-z0-9])?$'
- components.1.container.volumeMounts.1.name: Does not match pattern '^[a-z0-9]([-a-z0-9]*[a-z0-9])?$'
- components.3.name: Does not match pattern '^[a-z0-9]([-a-z0-9]*[a-z0-9])?$'

Expected behavior

odo push should succeed. It doesn't look like I gave an invalid name for storage component.

Any logs, error output, etc?

Devfile before odo storage create ```yaml schemaVersion: 2.1.0 # API Reference # https://docs.devfile.io/devfile/2.1.0/user-guide/api-reference.html # # Here another sample project that provides you with a basic express app # https://github.com/redhat-developer/devfile-sample/blob/master/devfile.yaml # # metadata - Metadata (optional) # name - Devfile name (optional) # version - Sematic version (optional) # displayName - Devfile display name (optional) # description - Devfile description (optional) # tags - Devfile tags (optional) # projectType - Devfile project type (optional) # language - Devfile language (optional) # # starterProjects - StarterProjects can be used as a starting point when bootstrapping new projects # name - Project name # git - Project's Git source # remotes - The remotes map which should be initialized in the git project # # components - List of the devworkspace components, such as editor and plugins, user-provided containers # name - Name that allows referencing the component from other elements # container - Allows adding and configuring devworkspace-related containers # image - The container image coordinates # memoryLimit - Container memory limit # mountSources - Toggles whether or not the project source code should be mounted in the component # volumeMounts - List of volumes mounts that should be mounted is this container # name - The volume mount name is the name of an existing Volume component # path - The path in the component container where the volume should be mounted # # commands - Predefined, ready-to-use, devworkspace-related commands # id - Identifier that allows referencing this command in composite commands, from a parent, or in events # exec - CLI Command executed in an existing component container # component - Describes component to which given action relates # commandLine - The actual command-line string # group - Defines the group this command is part of # kind - Kind of group the command is part of # isDefault - Identifies the default command for a given group kind metadata: name: camel-quarkus-ticker2log version: 1.0.0 displayName: Camel Quarkus Crypto Ticker description: Provides a simple crypto ticker based on Camel Quarkus. tags: [Camel, Quarkus, Maven, Java] projectType: maven language: java starterProjects: - name: ticker2log git: remotes: origin: "https://github.com/fuse-boosters/fuse-booster-camel-quarkus-ticker2log.git" checkoutFrom: revision: next components: - name: artemis container: image: quay.io/nessusio/activemq-artemis:2.16.0 env: - name: ANONYMOUS_LOGIN value: "true" endpoints: - name: amqtcp protocol: tcp targetPort: 61616 - name: tools container: image: quay.io/eclipse/che-java11-maven:nightly memoryLimit: 512Mi mountSources: true volumeMounts: - name: m2 path: /home/user/.m2 - name: m2 volume: {} commands: - id: mvn-package exec: component: tools commandLine: "mvn -Dmaven.repo.local=/home/user/.m2/repository package" group: kind: build isDefault: true - id: run exec: component: tools commandLine: "java -jar target/quarkus-app/quarkus-run.jar" group: kind: run isDefault: true - id: debug exec: component: tools commandLine: "java -Xdebug -Xrunjdwp:server=y,transport=dt_socket,address=${DEBUG_PORT},suspend=n -jar target/quarkus-app/quarkus-run.jar" group: kind: debug isDefault: true ```
devfile after doing odo storage create ```yaml commands: - exec: commandLine: mvn -Dmaven.repo.local=/home/user/.m2/repository package component: tools group: isDefault: true kind: build id: mvn-package - exec: commandLine: java -jar target/quarkus-app/quarkus-run.jar component: tools group: isDefault: true kind: run id: run - exec: commandLine: java -Xdebug -Xrunjdwp:server=y,transport=dt_socket,address=${DEBUG_PORT},suspend=n -jar target/quarkus-app/quarkus-run.jar component: tools group: isDefault: true kind: debug id: debug components: - container: endpoints: - name: amqtcp protocol: tcp targetPort: 61616 env: - name: ANONYMOUS_LOGIN value: "true" image: quay.io/nessusio/activemq-artemis:2.16.0 volumeMounts: - name: artemisStorage path: /var/lib/artemis-instance name: artemis - container: image: quay.io/eclipse/che-java11-maven:nightly memoryLimit: 512Mi mountSources: true volumeMounts: - name: m2 path: /home/user/.m2 - name: artemisStorage path: /var/lib/artemis-instance name: tools - name: m2 volume: size: 1Gi - name: artemisStorage volume: size: 2Gi metadata: description: Provides a simple crypto ticker based on Camel Quarkus. displayName: Camel Quarkus Crypto Ticker language: java name: camel-quarkus-ticker2log projectType: maven tags: - Camel - Quarkus - Maven - Java version: 1.0.0 schemaVersion: 2.1.0 starterProjects: - git: checkoutFrom: revision: next remotes: origin: https://github.com/fuse-boosters/fuse-booster-camel-quarkus-ticker2log.git name: ticker2log ```
kadel commented 3 years ago

the component name in devfile needs to be lowercased, so the storage name needs to be lowercase as well.

odo storage create artemisstorage --path=/var/lib/artemis-instance --size=2Gi should work.

odo should probably convert to lowercase the name automatically

$ odo storage create artemisStorage --path=/var/lib/artemis-instance --size=2Gi
Warning: Only lowercase letters allowed in storage name. Name will be automaticaly converted to "artemistorage"

/priority low

dharmit commented 3 years ago

odo should probably convert to lowercase the name automatically

@valaparthvi pointed me to Kubernetes docs for PV and it says:

Each PV contains a spec and status, which is the specification and status of the volume. The name of a PersistentVolume object must be a valid DNS subdomain name.

IMO, we need not convert to lowercase in odo. We could simply make the error more user friendly.

openshift-bot commented 3 years ago

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale. Stale issues rot after an additional 30d of inactivity and eventually close. Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

dharmit commented 3 years ago

/remove-lifecycle stale /good-first-issue

openshift-ci[bot] commented 3 years ago

@dharmit: This request has been marked as suitable for new contributors.

Please ensure the request meets the requirements listed here.

If this request no longer meets these requirements, the label can be removed by commenting with the /remove-good-first-issue command.

In response to [this](https://github.com/openshift/odo/issues/4981): >/remove-lifecycle stale >/good-first-issue Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
valaparthvi commented 2 years ago

/close closing in favor of v3, we will no longer have odo storage command in v3. https://github.com/kadel/odo/blob/odo-v3-proposal/docs/proposals/odo-v3-cli.md

openshift-ci[bot] commented 2 years ago

@valaparthvi: Closing this issue.

In response to [this](https://github.com/redhat-developer/odo/issues/4981#issuecomment-1032638648): >/close >closing in favor of v3, we will no longer have `odo storage` command in v3. Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.