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

🐛 Fixes after the rebase on the Syncer refactoring #1

Conversation

davidfestal
Copy link
Collaborator

@davidfestal davidfestal commented Jan 10, 2023

Signed-off-by: David Festal dfestal@redhat.com

Summary

Fixes after the rebase on the Syncer refactoring

Disclaimer

Still to be done after this PR is merged to kcp-dev#2214 :

  • Cleanup the logic that creates / updates the resource in upstream: updating the spec sub-resource doesn't make sense.
  • Rebase on top of main

Signed-off-by: David Festal <dfestal@redhat.com>
@davidfestal davidfestal changed the title Fixes after the rebase on the Syncer refactoring 🐛 Fixes after the rebase on the Syncer refactoring Jan 11, 2023
}
return informer.Lister(), nil
},
syncTargetName: syncTargetKey,

Choose a reason for hiding this comment

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

Hi, excuse me for interrupting.
I am very interesting to Upsync feature and am trying it on this branch.
Maybe, is this typo?
syncTargetName: syncTargetKey, => syncTargetName: syncTargetName,?
I'm facing an issue at /~https://github.com/davidfestal/kcp/blob/pr/bipuladh/2214/pkg/syncer/upsync/upsync_process.go#L60 that calculates the downstream namespace hash prefix but results in a different value from the actually used one.

@davidfestal
Copy link
Collaborator Author

Thanks @yana1205. I'll have a look into it.
I'm still polishing the fixes on the initial PR, and will update this PR when finished.

@davidfestal
Copy link
Collaborator Author

Yes, its seems a bug. I'll include the fix in my current changes

@yana1205
Copy link

Thanks! @davidfestal

@bipuladh bipuladh merged commit 8629bb5 into bipuladh:1980-storage-upsync-controller-syncer Jan 12, 2023
davidfestal pushed a commit that referenced this pull request Jan 13, 2023
bipuladh added a commit that referenced this pull request Jan 30, 2023
davidfestal pushed a commit that referenced this pull request Feb 7, 2023
davidfestal pushed a commit that referenced this pull request Feb 7, 2023
davidfestal pushed a commit that referenced this pull request Feb 7, 2023
davidfestal pushed a commit that referenced this pull request Feb 7, 2023
davidfestal pushed a commit that referenced this pull request Feb 8, 2023
davidfestal pushed a commit that referenced this pull request Feb 8, 2023
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.

3 participants