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

deploy should not need go environment #48

Closed
wants to merge 1 commit into from

Conversation

JingChen23
Copy link
Contributor

Signed-off-by: Chen Jing jingch@vmware.com

Description

This change contains 2 improvements.

  1. Change the imagePullPolicy to IfNotPresent to save unnecessary download.
  2. when user run ./deploy.sh install, they will call below functions in Makefile:
    deploy -> manifests -> controller-gen -> tidy
    But I don't think running tidy during deployment is the right behaviour.

We can let the developer run go tidy at debug or build stage, but never at deployment stage. This doesn't make sense.

This makes the user must install go 1.19 in the machine, this doesn't make sense neither

Signed-off-by: Chen Jing <jingch@vmware.com>
@codecov-commenter
Copy link

Codecov Report

❗ No coverage uploaded for pull request base (main@4746bcb). Click here to learn what that means.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main      #48   +/-   ##
=======================================
  Coverage        ?   45.57%           
=======================================
  Files           ?        6           
  Lines           ?      814           
  Branches        ?        0           
=======================================
  Hits            ?      371           
  Misses          ?      414           
  Partials        ?       29           
Flag Coverage Δ
unittests 45.57% <0.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@4everming
Copy link
Contributor

4everming commented Nov 24, 2022

Thanks for raising out this improvement. That's indeed what we'd like to do.
Please check this dependency in Makefile: deploy -> kustomize -> go-get-tool function.
It seems like it cannot eliminate the go sdk dependency with the code in this PR if that is what you'd like to do. I suggest adding a new user-facing target in Makefile to make CNSI can be deployed without go sdk.

@JingChen23 JingChen23 closed this Nov 25, 2022
@JingChen23 JingChen23 deleted the fix-makefile branch March 7, 2023 06:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants