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

[WIP]support manifest bundle. #57

Closed

Conversation

morvencao
Copy link
Contributor

@morvencao morvencao commented Feb 27, 2024

@@ -60,6 +63,15 @@ func (d *Resource) GetDeletionTimestamp() *metav1.Time {
return &metav1.Time{Time: d.Meta.DeletedAt.Time}
}

func (d *Resource) HasManifestBundle() bool {
if d.Manifest != nil {
if kind, ok := d.Manifest["kind"]; ok && kind == "ManifestWork" {
Copy link
Contributor

Choose a reason for hiding this comment

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

inspecting the payload for exactly and only one kind of resource smells to me because it's implicit. if Maestro wishes to support many resources in 1 API call, perhaps that should be part of the official API/openapi?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Apologies for any confusion. Currently, Maestro still only supports a single resource per API call.
This functionality in this PR is intended solely for internal use, specifically when Maestro accepts gRPC calls. Each API call should include a manifestwork, which, in turn, contains a bundle.
However, it's important to note that each API call is designed to handle only one manifestwork.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, I understand the desire to go from 1 to Many objects in the payload. I'm suggesting it's better to have that explicit in the API than an implicit string search like this.

Specifically, the Resource struct has the single Manifest attribute on it. In order to make this Many, perhaps a ResourceSet object is the correct one to introduce and it specifically has either many Resources (our Kind) or many Manifests.

@morvencao morvencao force-pushed the br_manifest_bundle_support branch 3 times, most recently from 63c8e9d to c5cc4ac Compare March 1, 2024 03:48
Signed-off-by: morvencao <lcao@redhat.com>
@morvencao morvencao force-pushed the br_manifest_bundle_support branch from c5cc4ac to f97d4e9 Compare March 1, 2024 04:59
@morvencao
Copy link
Contributor Author

appended test results:

# make test-integration
CGO_ENABLED=1 GOEXPERIMENT=boringcrypto go install -ldflags="" ./cmd/maestro
OCM_ENV=testing gotestsum --format short-verbose -- -p 1 -ldflags -s -v -timeout 1h  \
		./test/integration
I0301 04:57:37.425164  160941 framework.go:76] Initializing testing environment
I0301 04:57:37.683783  160941 framework.go:161] Using Mock OCM Authz Client
I0301 04:57:37.685950  160941 framework.go:199] Disabling Sentry error reporting
I0301 04:57:37.694039  160941 grpc_server.go:78] Serving gRPC service without TLS at 8090
PASS test/integration.TestConsumerGet (0.30s)
PASS test/integration.TestConsumerPost (0.21s)
PASS test/integration.TestConsumerPaging (0.32s)
PASS test/integration.TestControllerRacing (1.63s)
PASS test/integration.TestControllerReconcile (1.32s)
PASS test/integration.TestControllerSync (1.31s)
PASS test/integration.TestPulseServer (5.20s)
PASS test/integration.TestResourceGet (0.33s)
PASS test/integration.TestResourcePost (5.33s)
PASS test/integration.TestResourcePatch (0.28s)
PASS test/integration.TestResourcePaging (0.45s)
PASS test/integration.TestResourceListSearch (0.41s)
PASS test/integration.TestUpdateResourceWithRacingRequests (0.30s)
PASS test/integration.TestResourceFromGRPC (6.41s)
I0301 04:58:01.523191  160941 api_server.go:157] Web server terminated
PASS test/integration

DONE 14 tests in 1.054s

@morvencao
Copy link
Contributor Author

/assign @qiujian16 @clyang82 @skeeey

@morvencao
Copy link
Contributor Author

closing this in favor of #59

@morvencao morvencao closed this Mar 7, 2024
@morvencao morvencao deleted the br_manifest_bundle_support branch April 16, 2024 06:05
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.

2 participants