Closed xavier-hou closed 2 years ago
@hxcGit: GitHub didn't allow me to assign the following users: your_reviewer.
Note that only openyurtio members, repo collaborators and people who have commented on this issue/PR can be assigned. Additionally, issues/PRs can only have 10 assignees at the same time. For more information please see the contributor guide
Merging #92 (7477a12) into master (cf1f65a) will decrease coverage by
1.80%
. The diff coverage is27.48%
.
@@ Coverage Diff @@
## master #92 +/- ##
==========================================
- Coverage 25.27% 23.47% -1.81%
==========================================
Files 25 42 +17
Lines 2742 4772 +2030
==========================================
+ Hits 693 1120 +427
- Misses 1951 3500 +1549
- Partials 98 152 +54
Flag | Coverage Δ | |
---|---|---|
unittests | 23.47% <27.48%> (-1.81%) |
:arrow_down: |
Flags with carried forward coverage won't be shown. Click here to find out more.
Impacted Files | Coverage Δ | |
---|---|---|
...roller/uniteddeployment/uniteddeployment_update.go | 0.00% <0.00%> (ø) |
|
...er/uniteddeployment/uniteddeployment_controller.go | 0.91% <0.91%> (ø) |
|
...appmanager/controller/uniteddeployment/revision.go | 3.14% <3.14%> (ø) |
|
...teddeployment/uniteddeployment_controller_utils.go | 5.45% <5.45%> (ø) |
|
...anager/controller/uniteddeployment/pool_control.go | 16.82% <16.82%> (ø) |
|
...er/uniteddeployment/adapter/statefulset_adapter.go | 41.87% <41.87%> (ø) |
|
...ok/uniteddeployment/uniteddeployment_validation.go | 44.64% <44.64%> (ø) |
|
...bhook/uniteddeployment/uniteddeployment_webhook.go | 52.94% <52.94%> (ø) |
|
...ler/uniteddeployment/adapter/deployment_adapter.go | 66.66% <66.66%> (ø) |
|
...ontroller/uniteddeployment/adapter/adapter_util.go | 76.04% <76.04%> (ø) |
|
... and 7 more |
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.
@hxcGit the crd should put in chart directory too.
@hxcGit please update valid and mutate webhook for UnitedDeployment, and you can reference the newest webhook of other controllers in yurt-app-manager.
@huiwq1990 @rambohe-ch How can I add uniteddeployment
in charts/yurt-app-manager/templates/admission-webhooks/mutatingwebhookconfiguration.yaml
and charts/yurt-app-manager/templates/admission-webhooks/validatingwebhookconfiguration.yaml
?
Is there any command to do this automatically? Or I can add it manually?
@huiwq1990 @rambohe-ch How can I add
uniteddeployment
incharts/yurt-app-manager/templates/admission-webhooks/mutatingwebhookconfiguration.yaml
andcharts/yurt-app-manager/templates/admission-webhooks/validatingwebhookconfiguration.yaml
?Is there any command to do this automatically? Or I can add it manually?
@hxcGit Maybe you should add it manually.
@huiwq1990 Please take a look.
@rambohe-ch @huiwq1990
For deprecated warning, do you have any suggestions?
Maybe we can follow
Warning: apiextensions.k8s.io/v1beta1 CustomResourceDefinition is deprecated in v1.16+, unavailable in v1.22+; use apiextensions.k8s.io/v1 CustomResourceDefinition
@rambohe-ch @huiwq1990
For deprecated warning, do you have any suggestions?
Maybe we can follow
Warning: apiextensions.k8s.io/v1beta1 CustomResourceDefinition is deprecated in v1.16+, unavailable in v1.22+; use apiextensions.k8s.io/v1 CustomResourceDefinition
@hxcGit yes, we will deprecate UnitedDeployment
in v0.7.0, and unavailable in v1.1, only keep UnitedDeployment
in two versions for compatability.
@huiwq1990 @rambohe-ch How can I add
uniteddeployment
incharts/yurt-app-manager/templates/admission-webhooks/mutatingwebhookconfiguration.yaml
andcharts/yurt-app-manager/templates/admission-webhooks/validatingwebhookconfiguration.yaml
? Is there any command to do this automatically? Or I can add it manually?@hxcGit Maybe you should add it manually.
yes, chart should edit manually.
@hxcGit increate the chart version at charts/yurt-app-manager/Chart.yaml
, it will release a new version at https://github.com/openyurtio/openyurt-helm.
@hxcGit increate the chart version at
charts/yurt-app-manager/Chart.yaml
, it will release a new version at https://github.com/openyurtio/openyurt-helm.
change to version: 0.1.2
?
@hxcGit increate the chart version at
charts/yurt-app-manager/Chart.yaml
, it will release a new version at https://github.com/openyurtio/openyurt-helm.change to
version: 0.1.2
?
it's ok.
@hxcGit because codecov is reduced, we can not merge this pull request. maybe you need to make some unit tests for UnitedDeployment.
@hxcGit because codecov is reduced, we can not merge this pull request. maybe you need to make some unit tests for UnitedDeployment.
@rambohe-ch I found that most of the functions in UnitedDeployment Controller
rely on a K8s cluster. And I can't use sigs.k8s.io/controller-runtime/pkg/envtest
library in github action. Do you have any suggestions?
@hxcGit because codecov is reduced, we can not merge this pull request. maybe you need to make some unit tests for UnitedDeployment.
@rambohe-ch I found that most of the functions in
UnitedDeployment Controller
rely on a K8s cluster. And I can't usesigs.k8s.io/controller-runtime/pkg/envtest
library in github action. Do you have any suggestions?
@hxcGit It looks like the unit test coverage has increased now, so i will merge this pull request.
/lgtm /approve
[APPROVALNOTIFIER] This PR is APPROVED
This pull-request has been approved by: hxcGit, rambohe-ch
The full list of commands accepted by this bot can be found here.
The pull request process is described here
Signed-off-by: hxcGit houxc_mail@163.com
What type of PR is this?
What this PR does / why we need it:
We should reserve UnitedDeployment resource in yurt-app-manager for one or two versions time, not just rename it to YurtAppSet.
Which issue(s) this PR fixes:
Fixes #83, #62
Special notes for your reviewer:
Does this PR introduce a user-facing change?
other Note