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

Mock objects do nothing for Update interface #46

Open
yanweiguo opened this issue Nov 4, 2020 · 7 comments
Open

Mock objects do nothing for Update interface #46

yanweiguo opened this issue Nov 4, 2020 · 7 comments

Comments

@yanweiguo
Copy link
Member

For example:

// Update is a mock for the corresponding method.
func (m *MockHealthChecks) Update(ctx context.Context, key *meta.Key, arg0 *ga.HealthCheck) error {
	if m.UpdateHook != nil {
		return m.UpdateHook(ctx, key, arg0, m)
	}
	return nil
}

It does nothing if UpdateHook is nil. I would expect it to override what is stored in m.Objects[*key]. Do I miss something?

@spencerhance
Copy link
Member

I believe this is because "Update" is considered an additional method, and not part of the default methods. @bowei Do you remember if this is the case?

@spencerhance
Copy link
Member

Regardless this is something we deal with in ingress-gce with UpdateHooks, so this would be helpful there as well if we are able to fix it.

@yanweiguo
Copy link
Member Author

I believe this is because "Update" is considered an additional method, and not part of the default methods. @bowei Do you remember if this is the case?

Hmm, I think "Update" is used to update a GCE object in the legacy-cloud-provider package. For example: https://github.com/kubernetes/legacy-cloud-providers/blob/master/gce/gce_healthchecks.go#L80

@spencerhance
Copy link
Member

@yanweiguo I think there is some confusion here. Update is definitely used to update a GCE object, but not all resources support the Update method. Hence why it is considered an additional method: https://github.com/GoogleCloudPlatform/k8s-cloud-provider/blob/master/pkg/cloud/meta/meta.go#L239

I think is mainly a limitation of the current codebase

@yanweiguo
Copy link
Member Author

@yanweiguo I think there is some confusion here. Update is definitely used to update a GCE object, but not all resources support the Update method. Hence why it is considered an additional method: https://github.com/GoogleCloudPlatform/k8s-cloud-provider/blob/master/pkg/cloud/meta/meta.go#L239

I think is mainly a limitation of the current codebase

Ah thanks for explaining.

@spencerhance
Copy link
Member

@yanweiguo That isn't to say that we can't fix it though :)

@bowei
Copy link
Member

bowei commented May 10, 2023

Can you use the hooks to implement the functionality you need?

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

No branches or pull requests

3 participants