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

fix example deployment #151

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix example deployment #151

wants to merge 2 commits into from

Conversation

kvaps
Copy link
Contributor

@kvaps kvaps commented Sep 2, 2020

This PR fixes two errors in example app:

First is missing selector for the Deployment:

error validating data: ValidationError(Deployment.spec): missing required field "selector" in io.k8s.api.apps.v1.DeploymentSpec; if you choose to ignore these errors, turn validation off with --validate=false

Second is missing name for the Job (replaces generateName):

error: from tj-: cannot use generate name with apply

@kvaps kvaps force-pushed the fix-example branch 2 times, most recently from cc4610e to 7d9582d Compare September 2, 2020 21:45
@harsimranmaan
Copy link
Contributor

Are you using qbec show envKey | kubectl apply -f -? qbec apply should work with generateName

@kvaps
Copy link
Contributor Author

kvaps commented Sep 4, 2020

Yes, I'm going to use qbec with Argo CD, example setup: argoproj/argo-cd#2930 (comment)

@gotwarlost
Copy link
Contributor

I understand the changes to the deployment object but why doesn't generateName work? It is part of the k8s API spec and both kubectl and qbec honor it.

I think one of the unit tests relies on the object having a generated name which is why I cannot accept that particular change.

Happy to have you create a secondary test app under examples/ if you want to keep it tied to the qbec source tree

@kvaps
Copy link
Contributor Author

kvaps commented Sep 5, 2020

I don't know, may be because I was testing it on newest 1.19

@harsimranmaan
Copy link
Contributor

Kubectl has a defect with first apply on generateName. I'll dig up the details. Kubectl create is recommended

@harsimranmaan
Copy link
Contributor

Possibly this kubernetes/kubernetes#44501

@codecov-io
Copy link

Codecov Report

Merging #151 (fb6b612) into master (b72657c) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #151   +/-   ##
=======================================
  Coverage   73.41%   73.41%           
=======================================
  Files          51       51           
  Lines        4300     4300           
=======================================
  Hits         3157     3157           
  Misses        966      966           
  Partials      177      177           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b72657c...fb6b612. Read the comment docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants