Skip to content
This repository has been archived by the owner on Nov 8, 2023. It is now read-only.

Write the v1alpha2 webhook and enable it #102

Merged
merged 8 commits into from
Apr 3, 2023

Conversation

wangxye
Copy link
Member

@wangxye wangxye commented Mar 27, 2023

What type of PR is this?

/kind feature
/sig iot

What this PR does / why we need it:

Write the v1alpha2 webhook and enable it

Which issue(s) this PR fixes:

Fixes #101

@codecov
Copy link

codecov bot commented Mar 27, 2023

Codecov Report

Merging #102 (37fd9f2) into main (5801123) will increase coverage by 1.09%.
The diff coverage is 65.30%.

@@            Coverage Diff             @@
##             main     #102      +/-   ##
==========================================
+ Coverage   16.55%   17.64%   +1.09%     
==========================================
  Files           2        2              
  Lines         453      476      +23     
==========================================
+ Hits           75       84       +9     
- Misses        372      384      +12     
- Partials        6        8       +2     
Impacted Files Coverage Δ
pkg/webhook/edgex/edgex_webhook.go 60.36% <65.30%> (-5.55%) ⬇️

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

@LavenderQAQ LavenderQAQ self-requested a review March 31, 2023 05:44
)

var registerOnce sync.Once
// var registerOnce sync.Once
Copy link
Member

Choose a reason for hiding this comment

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

We need sync.Once to ensure that we only register once.

func RegisterFieldIndexers(fi client.FieldIndexer) error {
	var err error = nil
	registerOnce.Do(func() {
		register := func(obj client.Object, path string) error {
			return fi.IndexField(context.TODO(), obj, path, func(rawObj client.Object) []string {
				switch t := rawObj.(type) {
				case *v1alpha1.EdgeX:
					return []string{t.Spec.PoolName}
				case *v1alpha2.EdgeX:
					return []string{t.Spec.PoolName}
				default:
					return []string{}
				}
			})
		}
		if err = register(&v1alpha1.EdgeX{}, IndexerPathForNodepoolv1); err != nil {
			return
		}
		if err = register(&v1alpha2.EdgeX{}, IndexerPathForNodepoolv2); err != nil {
			return
		}
	})

	return err
}

main.go Outdated
@@ -24,6 +24,7 @@ import (

util "github.com/openyurtio/yurt-edgex-manager/controllers/utils"
edgexwebhook "github.com/openyurtio/yurt-edgex-manager/pkg/webhook/edgex"
Copy link
Member

Choose a reason for hiding this comment

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

The package name is changed to edgexwebhookv1alpha2

main.go Outdated
@@ -24,6 +24,7 @@ import (

util "github.com/openyurtio/yurt-edgex-manager/controllers/utils"
edgexwebhook "github.com/openyurtio/yurt-edgex-manager/pkg/webhook/edgex"
edgexwebhookV1 "github.com/openyurtio/yurt-edgex-manager/pkg/webhook/edgex/v1alpha1"
Copy link
Member

Choose a reason for hiding this comment

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

The package name is changed to edgexwebhookv1alpha1

"sigs.k8s.io/controller-runtime/pkg/client"
)

const (
IndexerPathForNodepool = "spec.poolname"
IndexerPathForNodepoolv1 = "spec.poolname"
IndexerPathForNodepoolv2 = "spec.poolname"
Copy link
Member

Choose a reason for hiding this comment

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

One problem is that the correct path should be spec.poolName. The two constants can be combined into one, because this represents the path to the index. Since v1alpha1 and v1alpha2 are both built using poolName, they do not differentiate between versions, and the names v1 and v2 are ambiguous.

*/

package edgex
package v1alpha2
Copy link
Member

Choose a reason for hiding this comment

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

Lack of copyright

Copy link
Member

@LavenderQAQ LavenderQAQ left a comment

Choose a reason for hiding this comment

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

/lgtm

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

Successfully merging this pull request may close these issues.

[feature request] Write the v1alpha2 webhook and enable it
2 participants