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

bug: Report detailed compatibility errors on incompatible SyncTarget.Status.ResourcesToSync #33

Open
davidfestal opened this issue Mar 22, 2023 · 6 comments
Assignees
Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/bug Categorizes issue or PR as related to a bug.

Comments

@davidfestal
Copy link
Member

davidfestal commented Mar 22, 2023

Describe the bug

When a resource to sync on a SyncTarget is incompatible, there are not details about the compatibility errors between the KCP schema and the SyncTarget schema.
That makes it quite hard to understand what happens.

Steps To Reproduce

  1. Create a SyncTarget that binds to the root:compute:kubernetes APIExport
  2. Install this SyncTarget in a Kubernetes physical cluster with an old release (1.21 for example)
  3. See that some of the resources in the SyncTarget.Status.ResourcesToSync list have the Incompatible state, but don't provide any more detail.

Expected Behaviour

We should have a compatibility detailed error message.
This should be possible, since the internal code used to do the compatibility check returns a list of errors (missing field, incompatible types, etc...)

Additional Context

Here is the code where the at least the err.Error() of the error coming from the schemacompat.EnsureStructuralSchemaCompatibility() call should be added in a detailed message on the ResourceToSync:

https://github.com/kcp-dev/kcp/blob/60ba17a1780879d4418ab8d1e90abc6ece65cbba/pkg/reconciler/workload/synctargetexports/synctargetcompatible_reconcile.go#L122-L128

@davidfestal davidfestal added kind/bug Categorizes issue or PR as related to a bug. good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. labels Mar 22, 2023
@rahulii
Copy link

rahulii commented Mar 24, 2023

/assign

@rahulii
Copy link

rahulii commented Mar 24, 2023

hey @davidfestal ,should I add a new field into ResourceToSync type that would indicate errors in case any..?

@davidfestal
Copy link
Member Author

I think we could do that.
Something similar to the Message in status conditions:

	// A human readable message indicating details about the compatibility errors 
	// This field may be empty.
	// +optional
	Message string `json:"message,omitempty"`

@davidfestal
Copy link
Member Author

@sttts wdyt about #33 ?

@mjudeikis
Copy link
Contributor

/transfer-issue contrib-tmc

1 similar comment
@mjudeikis
Copy link
Contributor

/transfer-issue contrib-tmc

@kcp-ci-bot kcp-ci-bot transferred this issue from kcp-dev/kcp Nov 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/bug Categorizes issue or PR as related to a bug.
Projects
Status: Backlog
Development

Successfully merging a pull request may close this issue.

3 participants