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

bugfix:argo create command not support namespace in yaml file #962

Merged
merged 1 commit into from
Mar 17, 2021

Conversation

zhaoyun006
Copy link
Contributor

modify before:
[root@k8s-master-1 argo-rollouts]# grep namespace examples/rollout-canary.yaml
namespace: base
[root@k8s-master-1 argo-rollouts]# kubectl argo create -f examples/rollout-canary.yaml
Error: the namespace of the provided object does not match the namespace sent on the request

modify after:
[root@k8s-master-1 argo-rollouts]# grep namespace examples/rollout-canary.yaml
namespace: base
[root@k8s-master-1 argo-rollouts]# kubectl argo create -f examples/rollout-canary.yaml
rollout.argoproj.io/rollout-canary created
[root@k8s-master-1 argo-rollouts]#

Signed-off-by: zhaozhiqiang [email protected]

Checklist:

  • Either (a) I've created an enhancement proposal and discussed it with the community, (b) this is a bug fix, or (c) this is a chore.
  • The title of the PR is (a) conventional, (b) states what changed, and (c) suffixes the related issues number. E.g. "fix(controller): Updates such and such. Fixes #1234".
  • I've signed the CLA.
  • I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged.
  • My builds are green. Try syncing with master if they are not.
  • My organization is added to USERS.md.

@zhaoyun006
Copy link
Contributor Author

@zcc35357949

@codecov-io
Copy link

Codecov Report

Merging #962 (adc9655) into master (01055cf) will decrease coverage by 0.01%.
The diff coverage is 71.42%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #962      +/-   ##
==========================================
- Coverage   81.60%   81.59%   -0.02%     
==========================================
  Files          99       99              
  Lines        8732     8738       +6     
==========================================
+ Hits         7126     7130       +4     
- Misses       1151     1152       +1     
- Partials      455      456       +1     
Impacted Files Coverage Δ
pkg/kubectl-argo-rollouts/cmd/create/create.go 66.03% <71.42%> (+0.01%) ⬆️

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 01055cf...adc9655. Read the comment docs.

Copy link
Contributor

@zcc35357949 zcc35357949 left a comment

Choose a reason for hiding this comment

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

when we use "kubectl create -f xxx -n myns", if the namespace in file is different from flag,kubectl will return not match error.
I think "kubectl argo rollouts create -f" should return like so in that situation.

func (c *CreateOptions) getNamespace(un unstructured.Unstructured) string {
ns := c.ArgoRolloutsOptions.Namespace()
if md, ok := un.Object["metadata"]; ok {
metadata := md.(map[string]interface{})
Copy link
Contributor

Choose a reason for hiding this comment

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

may panic here if metadata is not a map in the resource file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

not write matadata name is invalid resource file,
it's error

modify before:
[root@k8s-master-1 argo-rollouts]# grep namespace examples/rollout-canary.yaml
  namespace: base
[root@k8s-master-1 argo-rollouts]# kubectl argo create -f  examples/rollout-canary.yaml
Error: the namespace of the provided object does not match the namespace sent on the request

modify after:
[root@k8s-master-1 argo-rollouts]# grep namespace examples/rollout-canary.yaml
  namespace: base
[root@k8s-master-1 argo-rollouts]# kubectl argo create -f  examples/rollout-canary.yaml
rollout.argoproj.io/rollout-canary created
[root@k8s-master-1 argo-rollouts]#

Signed-off-by: zhaozhiqiang <[email protected]>
@zhaoyun006 zhaoyun006 force-pushed the bugfix-argo-rollouts branch from adc9655 to 9da9ab7 Compare January 26, 2021 04:19
@sonarcloud
Copy link

sonarcloud bot commented Jan 26, 2021

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
4.0% 4.0% Duplication

@jessesuen jessesuen merged commit 5f1c185 into argoproj:master Mar 17, 2021
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