Skip to content
This repository has been archived by the owner on Apr 8, 2021. It is now read-only.

Use architect 0.8.1 #37

Closed
wants to merge 10 commits into from
Closed

Use architect 0.8.1 #37

wants to merge 10 commits into from

Conversation

stone-z
Copy link
Contributor

@stone-z stone-z commented Mar 9, 2020

Includes fixes so that linting passes.

Currently fails due to a missing CloudProvider field which was also identified here. I can either fix that bug here or update this PR if that one goes in first

@stone-z stone-z requested a review from a team March 9, 2020 16:18
@stone-z stone-z self-assigned this Mar 9, 2020
Copy link
Contributor

@tfussell tfussell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@MarcelMue
Copy link
Contributor

Why is the build failing though?

@tfussell
Copy link
Contributor

tfussell commented Mar 9, 2020

Why is the build failing though?

There was a missing variable and the template error was ignored so it was silently truncating the output.

@MarcelMue
Copy link
Contributor

Okay build now fails very oddly - any idea what the cause could be?

@stone-z
Copy link
Contributor Author

stone-z commented Mar 10, 2020

Okay build now fails very oddly - any idea what the cause could be?

I think chart linting is kind of new, we probably haven't built ignition-operator since it got added

@stone-z
Copy link
Contributor Author

stone-z commented Mar 10, 2020

Running ct lint locally I don't get this issue

@MarcelMue
Copy link
Contributor

Yes - the error is super confusing to me:

 ✖︎ ignition-operator => (version: "0.0.1-67e7bf3548124c1320e8d11efd2b31d059897052", path: "helm/ignition-operator") > Error reading old chart version: Could not unmarshal 'Chart.yaml': yaml: unmarshal errors:
  line 3: cannot unmarshal !!seq into string

I don't understand how the unmarshal of that file fails - especially why does it refer to some old chart version?

@stone-z
Copy link
Contributor Author

stone-z commented Mar 10, 2020

If I set a version in values.yaml and re-lint, I get the error locally

@stone-z
Copy link
Contributor Author

stone-z commented Mar 10, 2020

Hey @kopiczko @yasn77 sorry for the ping but are we missing something obvious here regarding how to use the new chart linting in architect-orb?

@kopiczko
Copy link

It looks like it doesn't like square brackets in version: [[ .Version ]]. @yasn77 is there a way to disable that?

@stone-z have you managed to make linter work locally before templating?
Alternative could be linting after running architect helm template which I believe makes more sense.

@stone-z
Copy link
Contributor Author

stone-z commented Mar 10, 2020

Trying with a random string version gives me the same result:

✖︎ ignition-operator => (version: "aversion", path: "helm/ignition-operator") > Error reading old chart version: Could not unmarshal 'Chart.yaml': yaml: unmarshal errors:
  line 3: cannot unmarshal !!seq into string

@stone-z
Copy link
Contributor Author

stone-z commented Mar 10, 2020

This open issue makes me think it references the previous chart version

@kopiczko
Copy link

I'm trying to re-run the job with SSH and see what's wrong there

@kopiczko
Copy link

------------------------------------------------------------------------------------------------------------------------
 ✖︎ ignition-operator => (version: "0.0.1-67e7bf3548124c1320e8d11efd2b31d059897052", path: "helm/ignition-operator") > Error reading old chart version: Could not unmarshal 'Chart.yaml': yaml: unmarshal errors:
  line 3: cannot unmarshal !!seq into string
------------------------------------------------------------------------------------------------------------------------
68561456ea21:~/project# cat helm/ignition-operator/Chart.yaml
apiVersion: v1
name: ignition-operator
version: "0.0.1-67e7bf3548124c1320e8d11efd2b31d059897052"

So it's already templated and looks ok to me...

@kopiczko
Copy link

68561456ea21:~/project# ct lint --validate-maintainers=false --charts helm/ignition-operator
Linting charts...
Version increment checking disabled.
------------------------------------------------------------------------------------------------------------------------
 Configuration
------------------------------------------------------------------------------------------------------------------------
Remote: origin
TargetBranch: master
BuildId:
LintConf: /etc/ct/lintconf.yaml
ChartYamlSchema: /etc/ct/chart_schema.yaml
ValidateMaintainers: false
ValidateChartSchema: true
ValidateYaml: true
CheckVersionIncrement: false
ProcessAllCharts: false
Charts: [helm/ignition-operator]
ChartRepos: []
ChartDirs: [charts]
ExcludedCharts: []
HelmExtraArgs:
HelmRepoExtraArgs: []
Debug: false
Upgrade: false
SkipMissingValues: false
Namespace:
ReleaseLabel:
------------------------------------------------------------------------------------------------------------------------

------------------------------------------------------------------------------------------------------------------------
 Charts to be processed:
------------------------------------------------------------------------------------------------------------------------
 ignition-operator => (version: "0.0.1-67e7bf3548124c1320e8d11efd2b31d059897052", path: "helm/ignition-operator")
------------------------------------------------------------------------------------------------------------------------

$HELM_HOME has been configured at /root/.helm.
Not installing Tiller due to 'client-only' flag having been set
No requirements found in helm/ignition-operator/charts.
Linting chart 'ignition-operator => (version: "0.0.1-67e7bf3548124c1320e8d11efd2b31d059897052", path: "helm/ignition-operator")'
Validating /root/project/helm/ignition-operator/Chart.yaml...

Error!
Schema: /etc/ct/chart_schema.yaml
Data file: /root/project/helm/ignition-operator/Chart.yaml
Traceback (most recent call last):
  File "/usr/lib/python2.7/site-packages/yamale/command_line.py", line 29, in _validate
    yamale.validate(schema, data)
  File "/usr/lib/python2.7/site-packages/yamale/yamale.py", line 39, in validate
    schema.validate(d)
  File "/usr/lib/python2.7/site-packages/yamale/schema/schema.py", line 56, in validate
    raise ValueError(error_str)
ValueError:
Error validating data /root/project/helm/ignition-operator/Chart.yaml with schema /etc/ct/chart_schema.yaml
        description: Required field missing
        appVersion: Required field missing
        home: Required field missing

Traceback (most recent call last):
  File "/usr/bin/yamale", line 11, in <module>
    load_entry_point('yamale==1.8.0', 'console_scripts', 'yamale')()
  File "/usr/lib/python2.7/site-packages/yamale/command_line.py", line 112, in main
    _router(args.path, args.schema, args.cpu_num, args.parser)
  File "/usr/lib/python2.7/site-packages/yamale/command_line.py", line 96, in _router
    _validate_single(root, schema_name, parser)
  File "/usr/lib/python2.7/site-packages/yamale/command_line.py", line 68, in _validate_single
    _validate(s, yaml_path, parser)
  File "/usr/lib/python2.7/site-packages/yamale/command_line.py", line 36, in _validate
    raise ValueError('Validation failed!')
ValueError: Validation failed!
Error linting charts: Error processing charts
------------------------------------------------------------------------------------------------------------------------
 ✖︎ ignition-operator => (version: "0.0.1-67e7bf3548124c1320e8d11efd2b31d059897052", path: "helm/ignition-operator") > Error waiting for process: exit status 1
------------------------------------------------------------------------------------------------------------------------

This gives some more information:

ValueError:
Error validating data /root/project/helm/ignition-operator/Chart.yaml with schema /etc/ct/chart_schema.yaml
        description: Required field missing
        appVersion: Required field missing
        home: Required field missing

Also:

68561456ea21:~/project# cat /etc/ct/chart_schema.yaml
name: str()
home: str()
version: str()
appVersion: any(str(), num())
description: str()
keywords: list(str(), required=False)
sources: list(str(), required=False)
maintainers: list(include('maintainer'), required=False)
icon: str(required=False)
engine: str(required=False)
condition: str(required=False)
tags: str(required=False)
deprecated: bool(required=False)
kubeVersion: str(required=False)
annotations: map(str(), str(), required=False)
---
maintainer:
  name: str()
  email: str(required=False)
  url: str(required=False)

@yasn77
Copy link
Contributor

yasn77 commented Mar 10, 2020

I've raised a PR in architect-orb that disables version checking -> giantswarm/architect-orb#95

@kopiczko
Copy link

This pushed me a bit further:

68561456ea21:~/project# ct lint --validate-chart-schema=false --charts=helm/ignition-operator
Linting charts...
Version increment checking disabled.
------------------------------------------------------------------------------------------------------------------------
 Configuration
------------------------------------------------------------------------------------------------------------------------
Remote: origin
TargetBranch: master
BuildId:
LintConf: /etc/ct/lintconf.yaml
ChartYamlSchema: /etc/ct/chart_schema.yaml
ValidateMaintainers: true
ValidateChartSchema: false
ValidateYaml: true
CheckVersionIncrement: false
ProcessAllCharts: false
Charts: [helm/ignition-operator]
ChartRepos: []
ChartDirs: [charts]
ExcludedCharts: []
HelmExtraArgs:
HelmRepoExtraArgs: []
Debug: false
Upgrade: false
SkipMissingValues: false
Namespace:
ReleaseLabel:
------------------------------------------------------------------------------------------------------------------------

------------------------------------------------------------------------------------------------------------------------
 Charts to be processed:
------------------------------------------------------------------------------------------------------------------------
 ignition-operator => (version: "0.0.1-67e7bf3548124c1320e8d11efd2b31d059897052", path: "helm/ignition-operator")
------------------------------------------------------------------------------------------------------------------------

$HELM_HOME has been configured at /root/.helm.
Not installing Tiller due to 'client-only' flag having been set
No requirements found in helm/ignition-operator/charts.
Linting chart 'ignition-operator => (version: "0.0.1-67e7bf3548124c1320e8d11efd2b31d059897052", path: "helm/ignition-operator")'
Validating maintainers...
Error linting charts: Error processing charts
------------------------------------------------------------------------------------------------------------------------
 ✖︎ ignition-operator => (version: "0.0.1-67e7bf3548124c1320e8d11efd2b31d059897052", path: "helm/ignition-operator") > Chart doesn't have maintainers
------------------------------------------------------------------------------------------------------------------------
68561456ea21:~/project# ct lint --validate-maintainers=false --validate-chart-schema=false --charts=helm/ignition-operator
Linting charts...
Version increment checking disabled.
------------------------------------------------------------------------------------------------------------------------
 Configuration
------------------------------------------------------------------------------------------------------------------------
Remote: origin
TargetBranch: master
BuildId:
LintConf: /etc/ct/lintconf.yaml
ChartYamlSchema: /etc/ct/chart_schema.yaml
ValidateMaintainers: false
ValidateChartSchema: false
ValidateYaml: true
CheckVersionIncrement: false
ProcessAllCharts: false
Charts: [helm/ignition-operator]
ChartRepos: []
ChartDirs: [charts]
ExcludedCharts: []
HelmExtraArgs:
HelmRepoExtraArgs: []
Debug: false
Upgrade: false
SkipMissingValues: false
Namespace:
ReleaseLabel:
------------------------------------------------------------------------------------------------------------------------

------------------------------------------------------------------------------------------------------------------------
 Charts to be processed:
------------------------------------------------------------------------------------------------------------------------
 ignition-operator => (version: "0.0.1-67e7bf3548124c1320e8d11efd2b31d059897052", path: "helm/ignition-operator")
------------------------------------------------------------------------------------------------------------------------

$HELM_HOME has been configured at /root/.helm.
Not installing Tiller due to 'client-only' flag having been set
No requirements found in helm/ignition-operator/charts.
Linting chart 'ignition-operator => (version: "0.0.1-67e7bf3548124c1320e8d11efd2b31d059897052", path: "helm/ignition-operator")'
==> Linting helm/ignition-operator
[INFO] Chart.yaml: icon is recommended
[ERROR] templates/: render error in "ignition-operator/templates/pull-secret.yaml": template: ignition-operator/templates/pull-secret.yaml:9:31: executing "ignition-operator/templates/pull-secret.yaml" at <.Values.Installation.V1.Secret.Registry.PullSecret.DockerConfigJSON>: nil pointer evaluating interface {}.V1

Error: 1 chart(s) linted, 1 chart(s) failed
Error linting charts: Error processing charts
------------------------------------------------------------------------------------------------------------------------
 ✖︎ ignition-operator => (version: "0.0.1-67e7bf3548124c1320e8d11efd2b31d059897052", path: "helm/ignition-operator") > Error waiting for process: exit status 1
-

We should fix the job to use --charts and specify a single dir instead of --charts-dir. I don't know if disabling schema validation is good thing but it seems to work that way and then this next error...

@kopiczko
Copy link

I guess SkipMissingValues would fix above

@kopiczko
Copy link

kopiczko commented Mar 10, 2020

Yasser 11:09
I raised a PR last night giantswarm/architect-orb#95. It should fi the version issue

Oops he already mentioned that #37 (comment) :)

@yasn77
Copy link
Contributor

yasn77 commented Mar 10, 2020

Question is, do we care about those missing values? I mean the linter is doing its thing... The values are needed for a Helm Chart to be 'accurate'

If we don't need them, I think then it's better to introduce an option to either use a config file for ct or override options.

My initial PR for ct did have an option to use a config file... I can reintroduce it.

@kopiczko
Copy link

We can't avoid having missing .Values.Installation values. Maybe besides managed apps. So we need to adapt the job to work in those. I'd rather have on/off switch for that in the job for now (like ignore-missing-values: "true"). It's just a thing for charts templated with installations. I'd like to avoid copying over the same config to 90% of our repos.

@yasn77 yasn77 mentioned this pull request Mar 10, 2020
2 tasks
@yasn77
Copy link
Contributor

yasn77 commented Mar 10, 2020

Regarding this error:

==> Linting helm/ignition-operator
[INFO] Chart.yaml: icon is recommended
[ERROR] templates/: render error in "ignition-operator/templates/pull-secret.yaml": template: ignition-operator/templates/pull-secret.yaml:9:31: executing "ignition-operator/templates/pull-secret.yaml" at <.Values.Installation.V1.Secret.Registry.PullSecret.DockerConfigJSON>: nil pointer evaluating interface {}.V1

This is a renderer error and I couldn't see a way to bypass it with a switch to ct. However one of the benefits of ct is that you can easily test/lint the chart against multiple values files. So it is possible to prevent this error from happening by supplying a test values file. To do this create a new file helm/ignition-operator/ci/default-values.yaml with the following contents:

---
Installation:
  V1:
    Secret:
      Registry:
        PullSecret:
          DockerConfigJSON: SomeString

Lint should work fine after this:

# ct lint --validate-maintainers=false --check-version-increment=false --validate-chart-schema=false --charts ignition-operator
Linting charts...
Version increment checking disabled.
------------------------------------------------------------------------------------------------------------------------
 Configuration
------------------------------------------------------------------------------------------------------------------------
Remote: origin
TargetBranch: master
BuildId: 
LintConf: /etc/ct/lintconf.yaml
ChartYamlSchema: /etc/ct/chart_schema.yaml
ValidateMaintainers: false
ValidateChartSchema: false
ValidateYaml: true
CheckVersionIncrement: false
ProcessAllCharts: false
Charts: [ignition-operator]
ChartRepos: []
ChartDirs: [charts]
ExcludedCharts: []
HelmExtraArgs: 
HelmRepoExtraArgs: []
Debug: false
Upgrade: false
SkipMissingValues: false
Namespace: 
ReleaseLabel: 
------------------------------------------------------------------------------------------------------------------------

------------------------------------------------------------------------------------------------------------------------
 Charts to be processed:
------------------------------------------------------------------------------------------------------------------------
 ignition-operator => (version: "1.0", path: "ignition-operator")
------------------------------------------------------------------------------------------------------------------------

$HELM_HOME has been configured at /root/.helm.
Not installing Tiller due to 'client-only' flag having been set
No requirements found in ignition-operator/charts.
Linting chart 'ignition-operator => (version: "1.0", path: "ignition-operator")'

Linting chart with values file 'ignition-operator/ci/default-values.yaml'...

==> Linting ignition-operator
[INFO] Chart.yaml: icon is recommended

1 chart(s) linted, no failures
All charts linted successfully
------------------------------------------------------------------------------------------------------------------------
 ✔︎ ignition-operator => (version: "1.0", path: "ignition-operator")
------------------------------------------------------------------------------------------------------------------------

I'll continue to create a PR which allows the user to make the tests less strict. But skipping missing values will require a ci values file.

@yasn77
Copy link
Contributor

yasn77 commented Mar 11, 2020

@stone-z I would suggest closing this and waiting for the next orb release. I've also opened #42 which will help when the orb is released.

@stone-z stone-z mentioned this pull request Mar 17, 2020
@stone-z
Copy link
Contributor Author

stone-z commented Mar 17, 2020

Replaced by #47

@stone-z stone-z closed this Mar 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants