Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

odo deploy - devfile variable substitution does not occur in uri referenced deployment manifest yaml file #5451

Closed
ajm01 opened this issue Feb 10, 2022 · 0 comments · Fixed by #5711
Assignees
Labels
kind/bug Categorizes issue or PR as related to a bug. priority/High Important issue; should be worked on before any other issues (except priority/Critical issue(s)).

Comments

@ajm01
Copy link

ajm01 commented Feb 10, 2022

/kind bug

What versions of software are you using?

Operating System:

Distributor ID: Ubuntu
Description:    Ubuntu 20.04.3 LTS
Release:        20.04
Codename:       focal

Output of odo version:

$ odo version
odo v2.5.0 (724f16e68)

How did you run odo exactly?

$ odo deploy

with the following devfile variables defined:

variables:
  container_image: 'docker.io/ajymau/asi-outer-loop:app-provided'
  appname: 'test-inline'

and the following component (and related commands) defined:

  - name: myk8sdeploy
    kubernetes:
      uri: ./app-deploy.yaml

app-deploy.yaml contains substitution placeholders for the vars defined in the devfile:

apiVersion: apps.openliberty.io/v1beta2
kind: OpenLibertyApplication
metadata:
  name: {{appname}}
spec:
  # Add fields here.
  applicationVersion: 1.0.0
  applicationImage: {{container_image}}
  service:
    type: ClusterIP
    port: 9080
    annotations:
      prometheus.io/scrape: 'true'
  probes:
    readiness:
      failureThreshold: 12
      httpGet:
        path: /health/ready
        port: 9080
      initialDelaySeconds: 5
      periodSeconds: 2
      timeoutSeconds: 1
    liveness:
      failureThreshold: 12
      httpGet:
        path: /health/live
        port: 9080
      initialDelaySeconds: 5
      periodSeconds: 2
  expose: true
  route:
    # Ingress required entries.
    pathType: 'Prefix'
    path: '/'

Actual behavior

the app-deploy.yaml is used as is to do the deploy - no substitution occurs and the deploy fails because the names cannot be parsed:

$ odo deploy
.
.
..
173357979467: Layer already exists
app-provided: digest: sha256:40a963755d2202edf123e5868227dc64490a95302855ad20a9a607ced80d92f8 size: 4706
 ✗  command execution failed: error converting YAML to JSON: yaml: invalid map key: map[interface {}]interface {}{"appname":interface {}(nil)}
 $

Expected behavior

variable substitution occurs first and the deploy occurs next, successfully

Note: if i inline the contents of app-deploy.yaml in the devfile the substitutions occur as expected. This should happen as well for a uri specified deployment manifest yaml.

Any logs, error output, etc?

@openshift-ci openshift-ci bot added the kind/bug Categorizes issue or PR as related to a bug. label Feb 10, 2022
@kadel kadel added priority/High Important issue; should be worked on before any other issues (except priority/Critical issue(s)). v2 Issue or PR that applies to the v2 of odo v3 labels Feb 11, 2022
@kadel kadel removed the v3 label Mar 24, 2022
@kadel kadel removed the v2 Issue or PR that applies to the v2 of odo label Mar 29, 2022
@kadel kadel removed this from odo v3-alpha1 Mar 29, 2022
@kadel kadel moved this to Todo in odo v3-beta1 Apr 27, 2022
@joshira-rh joshira-rh moved this from Todo to In Progress in odo v3-beta1 Apr 28, 2022
@rm3l rm3l moved this from In Progress to For Review in odo v3-beta1 May 2, 2022
openshift-merge-robot pushed a commit that referenced this issue May 5, 2022
#5451) (#5711)

* Add integration test highlighting the issue

The devfile used is the same as devfile-deploy.yaml,
except that the the Kubernetes component is referenced via the URI.
This is why the same integration test logic is being reused here.

* Add higher-level function in 'libdevfile' allowing to load component resource manifest and substitute variables

This works with both Inlined or Uri resources.

Notes:
- Ideally, it would make more sense to rely on the same logic used in Devfile to
  substitute variables (defined in the 'variables' package),
  but this function is not exported;
  and the exported ones substitute variables only in the URI name,
  not in the content itself (it is not fetched),
  which is actually the issue we are trying to solve here.

* Switch to using 'uri' Kubernetes components in test Devfiles

This seems to be a much more realistic case when referencing external
Kubernetes manifests.

Notes:
Some tests, like for `odo deploy`, still test 'Inlined' manifests.

* Pass the component name to 'GetComponentResourceManifestContentWithVariablesResolved'

As suggested in review, this makes more sense
now that we are passing the entire devfile object

* Rename 'GetComponentResourceManifestContentWithVariablesResolved' with 'GetK8sManifestWithVariablesSubstituted'

Previous name was a bit long ^^

* Remove extra parentheses in error string returned by 'ComponentsWithSameNameError#Error', as suggested in review

* Revert "Switch to using 'uri' Kubernetes components in test Devfiles"

This reverts commit df1f19e.

This will be done in a separate PR, with ideally
changes that should allow to use the same set of
tests for testing both Inlined and
URI-referenced manifests.
Repository owner moved this from For Review to Done in odo v3-beta1 May 5, 2022
cdrage pushed a commit to cdrage/odo that referenced this issue Aug 31, 2022
redhat-developer#5451) (redhat-developer#5711)

* Add integration test highlighting the issue

The devfile used is the same as devfile-deploy.yaml,
except that the the Kubernetes component is referenced via the URI.
This is why the same integration test logic is being reused here.

* Add higher-level function in 'libdevfile' allowing to load component resource manifest and substitute variables

This works with both Inlined or Uri resources.

Notes:
- Ideally, it would make more sense to rely on the same logic used in Devfile to
  substitute variables (defined in the 'variables' package),
  but this function is not exported;
  and the exported ones substitute variables only in the URI name,
  not in the content itself (it is not fetched),
  which is actually the issue we are trying to solve here.

* Switch to using 'uri' Kubernetes components in test Devfiles

This seems to be a much more realistic case when referencing external
Kubernetes manifests.

Notes:
Some tests, like for `odo deploy`, still test 'Inlined' manifests.

* Pass the component name to 'GetComponentResourceManifestContentWithVariablesResolved'

As suggested in review, this makes more sense
now that we are passing the entire devfile object

* Rename 'GetComponentResourceManifestContentWithVariablesResolved' with 'GetK8sManifestWithVariablesSubstituted'

Previous name was a bit long ^^

* Remove extra parentheses in error string returned by 'ComponentsWithSameNameError#Error', as suggested in review

* Revert "Switch to using 'uri' Kubernetes components in test Devfiles"

This reverts commit df1f19e.

This will be done in a separate PR, with ideally
changes that should allow to use the same set of
tests for testing both Inlined and
URI-referenced manifests.
@rm3l rm3l added the v3 label Oct 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug. priority/High Important issue; should be worked on before any other issues (except priority/Critical issue(s)).
Projects
No open projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants