Feat: support set labels for env #4422
Conversation
huanghantao
requested review from
Somefive,
zzxwill,
StevenLeiZhang,
barnettZQG,
wonderflow and
leejanee
as code owners
huanghantao
force-pushed
the
set-env-labels
branch
from
2533bde
to
e84c5f4
Compare
Codecov Report
@@ Coverage Diff @@
## master #4422 +/- ##
===========================================
+ Coverage 51.27% 61.48% +10.20%
===========================================
Files 325 347 +22
Lines 33400 34278 +878
===========================================
+ Hits 17126 21075 +3949
+ Misses 13981 10453 -3528
- Partials 2293 2750 +457
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
huanghantao
force-pushed
the
set-env-labels
branch
from
e84c5f4
to
5a19246
Compare
Signed-off-by: codinghuang <codinghuang@qq.com>
huanghantao
force-pushed
the
set-env-labels
branch
from
5a19246
to
25558c1
Compare
pkg/oam/util/helper.go
Outdated
| return nil, fmt.Errorf("the label string can't be empty") | ||
| } | ||
| labels := map[string]string{} | ||
| labelStrings := strings.Split(labelString, ",") |
There was a problem hiding this comment.
Great, but if you are to write the label parser, remember to remove spaces inside the string first to prevent subtle problems.
Example in k8s.io/cli-runtime:
https://github.com/kubernetes/cli-runtime/blob/4661c56a3b88e2288cba9b858718fca45f522e02/pkg/resource/builder.go#L480-L481
But I would suggest that you take a look at k8s.io/apimachinery/pkg/labels for some existing parsers, instead of writing your own:
https://github.com/kubernetes/apimachinery/blob/cff14a57b273bd82d64efc85aa0835aaf847a3ef/pkg/labels/labels.go#L146-L148
use k8s.io/apimachinery/pkg/labels Signed-off-by: codinghuang <codinghuang@qq.com>
| @@ -131,6 +131,7 @@ func NewEnvDeleteCommand(c common.Args, ioStreams cmdutil.IOStreams) *cobra.Comm | |||
|
|
|||
| // NewEnvSetCommand creates `env set` command for setting current environment | |||
| func NewEnvSetCommand(c common.Args, ioStreams cmdutil.IOStreams) *cobra.Command { | |||
There was a problem hiding this comment.
Is this Feature only for vela CLI? Does api-server need update api?
Somefive
added
the
backport release-1.5
|
Successfully created backport PR #4506 for |
Signed-off-by: codinghuang codinghuang@qq.com
Description of your changes
Fixes #4180
I have:
make reviewableto ensure this PR is ready for review.backport release-x.ylabels to auto-backport this PR if necessary.How has this code been tested
Manually tested with possible user inputs. Unit tests for the util function are added.
Special notes for your reviewer
change
--labelto--labels, so we can set multiple labels: