Skip to content

Commit

Permalink
fixes various bugs discovered after manual testing
Browse files Browse the repository at this point in the history
renames ostree to libostree to better represent what it does
implements GCancellable to allow c functions to respect go context.Context
  • Loading branch information
Kyle Ishie committed Jan 12, 2024
1 parent aac24f2 commit 9478fb4
Show file tree
Hide file tree
Showing 23 changed files with 344 additions and 167 deletions.
File renamed without changes.
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@ package ostree

import (
"context"
"github.com/google/go-containerregistry/pkg/authn"
"github.com/google/go-containerregistry/pkg/name"
"github.com/google/go-containerregistry/pkg/v1/remote"
"github.com/spf13/cobra"
"go.ciq.dev/beskar/cmd/beskarctl/ctl"
"go.ciq.dev/beskar/pkg/orasostree"
Expand All @@ -22,7 +24,10 @@ var (
return ctl.Err("a directory must be specified")
}

if err := orasostree.PushOSTreeRepository(context.Background(), dir, ctl.Repo(), jobCount, name.WithDefaultRegistry(ctl.Registry())); err != nil {
repoPusher := orasostree.NewOSTreeRepositoryPusher(context.Background(), dir, ctl.Repo(), jobCount)
repoPusher = repoPusher.WithNameOptions(name.WithDefaultRegistry(ctl.Registry()))
repoPusher = repoPusher.WithRemoteOptions(remote.WithAuthFromKeychain(authn.DefaultKeychain))
if err := repoPusher.Push(); err != nil {
return ctl.Errf("while pushing ostree repository: %s", err)
}
return nil
Expand Down
8 changes: 8 additions & 0 deletions internal/pkg/repository/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,14 @@ func (rh *RepoHandler) DeleteManifest(ref string) (errFn error) {
return remote.Delete(namedRef, rh.Params.RemoteOptions...)
}

func (rh *RepoHandler) PullManifest(ref string) (errFn error) {
namedRef, err := name.ParseReference(ref, rh.Params.NameOptions...)
if err != nil {
return err
}
return remote.Delete(namedRef, rh.Params.RemoteOptions...)
}

func (rh *RepoHandler) SyncArtifact(ctx context.Context, name string, timeout time.Duration) (chan error, func() error) {
errCh := make(chan error, 1)

Expand Down
4 changes: 2 additions & 2 deletions internal/plugins/ostree/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,12 @@ func (p *Plugin) AddRemote(ctx context.Context, repository string, properties *a
return p.repositoryManager.Get(ctx, repository).AddRemote(ctx, properties)
}

func (p *Plugin) SyncRepository(ctx context.Context, repository string, request *apiv1.OSTreeRepositorySyncRequest) (err error) {
func (p *Plugin) SyncRepository(ctx context.Context, repository string, properties *apiv1.OSTreeRepositorySyncRequest) (err error) {
if err := checkRepository(repository); err != nil {
return err
}

return p.repositoryManager.Get(ctx, repository).SyncRepository(ctx, request)
return p.repositoryManager.Get(ctx, repository).SyncRepository(ctx, properties)
}

func (p *Plugin) GetRepositorySyncStatus(ctx context.Context, repository string) (syncStatus *apiv1.SyncStatus, err error) {
Expand Down
2 changes: 1 addition & 1 deletion internal/plugins/ostree/pkg/libostree/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ ostree is a wrapper around [libostree](https://github.com/ostreedev/ostree) that
1. A minimal glib implementation exists within the ostree pkg. This is to avoid a dependency on glib for the time being.
- This implementation is not complete and will be expanded as needed.
- The glib implementation is not intended to be used outside of the ostree pkg.
- `GCancellable` is not implemented. Just send nil.
- `GCancellable` is not implemented on some functions. If the func accepts a context.Context it most likely implements a GCancellable.
2. Not all of libostree is wrapped. Only the parts that are needed for beskar are wrapped. Which is basically everything
need to perform pull operations.
- `OstreeAsyncProgress` is not implemented. Just send nil.
Expand Down
2 changes: 1 addition & 1 deletion internal/plugins/ostree/pkg/libostree/errors.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package ostree
package libostree

type Err string

Expand Down
6 changes: 4 additions & 2 deletions internal/plugins/ostree/pkg/libostree/glib_helpers.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package ostree
package libostree

// #cgo pkg-config: glib-2.0 gobject-2.0
// #include <glib.h>
Expand All @@ -7,7 +7,9 @@ package ostree
// #include <stdlib.h>
// #include "glib_helpers.go.h"
import "C"
import "errors"
import (
"errors"
)

// GoError converts a C glib error to a Go error.
// The C error is freed after conversion.
Expand Down
2 changes: 1 addition & 1 deletion internal/plugins/ostree/pkg/libostree/options.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package ostree
package libostree

// #cgo pkg-config: ostree-1 glib-2.0 gobject-2.0
// #include <stdlib.h>
Expand Down
2 changes: 1 addition & 1 deletion internal/plugins/ostree/pkg/libostree/ostree.go
Original file line number Diff line number Diff line change
@@ -1 +1 @@
package ostree
package libostree
18 changes: 14 additions & 4 deletions internal/plugins/ostree/pkg/libostree/pull.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package ostree
package libostree

// #cgo pkg-config: ostree-1 glib-2.0 gobject-2.0
// #include <stdlib.h>
Expand All @@ -13,7 +13,7 @@ import (

// Pull pulls refs from the named remote.
// Returns an error if the refs could not be fetched.
func (r *Repo) Pull(_ context.Context, remote string, opts ...Option) error {
func (r *Repo) Pull(ctx context.Context, remote string, opts ...Option) error {
cremote := C.CString(remote)
defer C.free(unsafe.Pointer(cremote))

Expand All @@ -22,14 +22,24 @@ func (r *Repo) Pull(_ context.Context, remote string, opts ...Option) error {

var cErr *C.GError

cCancel := C.g_cancellable_new()
go func() {
for {
select {
case <-ctx.Done():
C.g_cancellable_cancel(cCancel)
return
}
}
}()

// Pull refs from remote
// TODO: Implement cancellable so that we can cancel the pull if needed.
if C.ostree_repo_pull_with_options(
r.native,
cremote,
options,
nil,
nil,
cCancel,
&cErr,
) == C.gboolean(0) {
return GoError(cErr)
Expand Down
17 changes: 16 additions & 1 deletion internal/plugins/ostree/pkg/libostree/pull_test.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package ostree
package libostree

import (
"context"
Expand Down Expand Up @@ -103,6 +103,21 @@ func TestRepo_Pull(t *testing.T) {
assert.Equal(t, remotes, []string{remoteName})
})

t.Run("should cancel pull", func(t *testing.T) {
ctx, cancel := context.WithCancel(context.Background())
cancel()

err := repo.Pull(
ctx,
remoteName,
Flags(Mirror|TrustedHttp),
)
assert.Error(t, err)
if err == nil {
assert.Failf(t, "failed to cancel pull", "err: %s", err.Error())
}
})

//TODO: Repeat the following tests for only a specific ref
t.Run("should pull entire repo", func(t *testing.T) {
err := repo.Pull(
Expand Down
4 changes: 2 additions & 2 deletions internal/plugins/ostree/pkg/libostree/repo.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package ostree
package libostree

// #cgo pkg-config: ostree-1 glib-2.0 gobject-2.0
// #include <stdlib.h>
Expand Down Expand Up @@ -85,7 +85,7 @@ func fromNative(cRepo *C.OstreeRepo) *Repo {

// Let the GB trigger free the cRepo for us when repo is freed.
runtime.SetFinalizer(repo, func(r *Repo) {
C.free(unsafe.Pointer(cRepo))
C.free(unsafe.Pointer(r.native))
})

return repo
Expand Down
Loading

0 comments on commit 9478fb4

Please sign in to comment.