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

feat(webhook): add cstor-webhook using openebs/api changes #8

Merged
merged 5 commits into from
Mar 9, 2020

Conversation

shubham14bajpai
Copy link
Contributor

Signed-off-by: shubham shubham.bajpai@mayadata.io

This PR adds webhook server code to validate cstor related create, update and delete requests.

@shubham14bajpai shubham14bajpai changed the title WIP feat(webhook): add cstor-webhook using openebs/api changes feat(webhook): add cstor-webhook using openebs/api changes Mar 9, 2020
}

// GetClusterConfig return the config for k8s.
func getClusterConfig(kubeconfig string) (*rest.Config, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

this func is getting used every where let's have in a single place and use it instead , we can do this change later in upcoming PRs.
Better to use below func helpful in case of outofcluster runs as binary

// GetClusterConfig return the config for k8s.
func getClusterConfig(kubeconfig string) (*rest.Config, error) {

	if kubeconfig != "" {
		return clientcmd.BuildConfigFromFlags("", kubeconfig)
	}
	return rest.InClusterConfig()
}

Copy link
Contributor

@prateekpandey14 prateekpandey14 Mar 9, 2020

Choose a reason for hiding this comment

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

Above change can be done later, we are using above getConfig func in case of other controller like cvc-controller, cspc-controller .

cstor.PoolRaidz: 3,
cstor.PoolRaidz2: 6,
}
)
Copy link
Contributor

Choose a reason for hiding this comment

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

ToDo: Fix the min validation in next PR. As for raidz1 and raidz2 the min is 2^n+1 and 2^n+2 respectively,

Signed-off-by: shubham <shubham.bajpai@mayadata.io>
Signed-off-by: shubham <shubham.bajpai@mayadata.io>
Signed-off-by: shubham <shubham.bajpai@mayadata.io>
Signed-off-by: shubham <shubham.bajpai@mayadata.io>
Copy link
Contributor

@mittachaitu mittachaitu left a comment

Choose a reason for hiding this comment

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

Provided few comments

cmd/webhook/main.go Outdated Show resolved Hide resolved
pkg/webhook/configuration.go Outdated Show resolved Hide resolved
cmd/webhook/main.go Outdated Show resolved Hide resolved
// transformation function lists to upgrade webhook resources
transformSecret = []transformSecretFunc{}
transformSvc = []transformSvcFunc{}
transformConfig = []transformConfigFunc{}
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we need to include that upgrade related functions? Or the user will be always in the latest version so not required to have them is that the idea?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the framework for future upgrades like adding and removing rules. This code will not affect the current state.

) error {

// Fetch our namespace
openebsNamespace, err := getOpenebsNamespace()
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe in the next PR's, we can set one global variable for the namespace and we will reuse it everywhere it's not good to fetch the value from env.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will take this in upcoming PR.

blockDeviceMap := map[string]bool{}
addedBlockDevices := map[string]bool{}
for _, poolSpec := range cspc.Spec.Pools {
for _, raidGroup := range poolSpec.DataRaidGroups {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a TODO task for WriteCacheRaidGroups?

pkg/webhook/bd_replacement.go Outdated Show resolved Hide resolved
}
if _, ok := SupportedPRaidType[cstor.PoolType(poolConfig.DataRaidGroupType)]; !ok {
return false, fmt.Sprintf("unsupported raid type '%s' specified", poolConfig.DataRaidGroupType)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This check is also required

if len(poolValidator.poolSpec.WriteCacheRaidGroups) != 0 {
if _, ok :=  SupportedPRaidType[cstor.PoolType(PoolConfig.WriteCacheRaidType)] == "" ; !ok {
  return false, msg
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a new schema validation will take all new schema validation in separate PR.

}

if rgType != string(cstor.PoolStriped) {
if len(raidGroup.BlockDevices) != SupportedPRaidType[cstor.PoolType(rgType)] {
Copy link
Contributor

Choose a reason for hiding this comment

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

TODO: Update check in next PR's

pkg/webhook/cspc.go Outdated Show resolved Hide resolved
Signed-off-by: shubham <shubham.bajpai@mayadata.io>
Copy link
Contributor

@mittachaitu mittachaitu left a comment

Choose a reason for hiding this comment

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

changes are good with existing migration...

@sonasingh46 sonasingh46 merged commit 7f13aff into openebs-archive:master Mar 9, 2020
@shubham14bajpai shubham14bajpai deleted the webhook branch March 10, 2020 04:19
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