Skip to content

Commit

Permalink
refactor go binding module/client to use struct
Browse files Browse the repository at this point in the history 
Problem: we currently pass around a ctx to interact with all
go binding module or client functions, and provide the developer
user with context that we are using the clients via function
names (e.g., ReapiCliMatchAllocate).
Solution: we can save the context alongside a struct that is
returned with XNew(), and then rename the functions to not
include the extra information (e.g., cli.MatchAllocate() implies
MatchAllocate for the ReapiClient{ctx:ctx} that it is created
from. This also removes the defer from the Go bindings, which
should not be needed as the CString without new is allocated
to the stack.

Signed-off-by: vsoch <[email protected]>
  • Loading branch information
@vsoch
vsoch committed Aug 23, 2023
1 parent 00673b9 commit 9089091
Show file tree
Hide file tree
Showing 6 changed files with 102 additions and 96 deletions.
8 changes: 7 additions & 1 deletion README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ export WITH_GO=yes
make
```

To run just one test, you can sd into t
To run just one test, you can cd into t

```bash
$ ./t9001-golang-basic.t
Expand All @@ -129,6 +129,12 @@ ok 2 - match allocate 2 slots: 2 sockets: 5 cores 1 gpu 6 memory
1..2
```

To run full tests (more robust and mimics what happens in CI) you can do:

```bash
make check
```

##### Flux Instance

The examples below walk through exercising functioning flux-sched modules (i.e.,
Expand Down
96 changes: 45 additions & 51 deletions resource/reapi/bindings/go/src/fluxcli/reapi_cli.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,25 +16,25 @@ package fluxcli
import "C"
import (
"fmt"
"unsafe"
)

type (
ReapiCtx C.struct_reapi_cli_ctx_t

// ReapiClient is a flux resource API client
// it holds a context that is required for most interactinos
ReapiClient struct {
ctx *ReapiCtx
}
)

// NewReapiCli creates a new resource API client
// reapi_cli_ctx_t *reapi_cli_new ();
func NewReapiCli() *ReapiCtx {
return (*ReapiCtx)(C.reapi_cli_new())
func NewReapiClient() *ReapiClient {
ctx := (*ReapiCtx)(C.reapi_cli_new())
return &ReapiClient{ctx: ctx}
}

// // ReapiCliGetNode retruns a resource node
// This function is currently not being used
// func ReapiCliGetNode(ctx *ReapiCtx) string {
// return C.GoString(C.reapi_cli_get_node((*C.struct_reapi_cli_ctx)(ctx)))
// }

// Given an integer return code, convert to go error
// Also provide a meaningful string to the developer user
func retvalToError(code int, message string) error {
Expand All @@ -44,31 +44,33 @@ func retvalToError(code int, message string) error {
return fmt.Errorf(message+" %d", code)
}

// ReapiCliDestroy destroys a resource API context
// HasContext exposes the private ctx, telling the caller if it is set
func (cli *ReapiClient) HasContext() bool {
return cli.ctx != nil
}

// Destroy destroys a resource API context
// void reapi_cli_destroy (reapi_cli_ctx_t *ctx);
func ReapiCliDestroy(ctx *ReapiCtx) {
C.reapi_cli_destroy((*C.struct_reapi_cli_ctx)(ctx))
func (cli *ReapiClient) Destroy() {
C.reapi_cli_destroy((*C.struct_reapi_cli_ctx)(cli.ctx))
}

// ReapiCliInit initializes a new resource API context
// InitContext initializes a new resource API context
// int reapi_cli_initialize (reapi_cli_ctx_t *ctx, const char *jgf);
func ReapiCliInit(ctx *ReapiCtx, jgf string, options string) (err error) {
func (cli *ReapiClient) InitContext(jgf string, options string) (err error) {

jobgraph := C.CString(jgf)
defer C.free(unsafe.Pointer(jobgraph))

opts := C.CString(options)
defer C.free(unsafe.Pointer(opts))
fluxerr := (int)(
C.reapi_cli_initialize(
(*C.struct_reapi_cli_ctx)(ctx), jobgraph, (opts),
(*C.struct_reapi_cli_ctx)(cli.ctx), jobgraph, (opts),
),
)

return retvalToError(fluxerr, "issue initializing resource api client")
}

// ReapiCliMatchAllocate matches a jobspec to the best resources, either
// MatchAllocate matches a jobspec to the best resources, either
// allocating or reserved them. The best resources are determined by the
// match policy.
//
Expand All @@ -94,18 +96,14 @@ func ReapiCliInit(ctx *ReapiCtx, jgf string, options string) (err error) {
// const char *jobspec, const uint64_t jobid,
// bool *reserved,
// char **R, int64_t *at, double *ov);
func ReapiCliMatchAllocate(
ctx *ReapiCtx,
func (cli *ReapiClient) MatchAllocate(
orelse_reserve bool,
jobspec string,
) (reserved bool, allocated string, at int64, overhead float64, jobid uint64, err error) {
var r = C.CString("")
defer C.free(unsafe.Pointer(r))

spec := C.CString(jobspec)
defer C.free(unsafe.Pointer(spec))

fluxerr := (int)(C.reapi_cli_match_allocate((*C.struct_reapi_cli_ctx)(ctx),
fluxerr := (int)(C.reapi_cli_match_allocate((*C.struct_reapi_cli_ctx)(cli.ctx),
(C.bool)(orelse_reserve),
spec,
(*C.ulong)(&jobid),
Expand All @@ -121,7 +119,7 @@ func ReapiCliMatchAllocate(

}

// ReapiCliUpdateAllocate updates the resource state with R.
// UpdateAllocate updates the resource state with R.
//
// \param ctx reapi_cli_ctx_t context object
// \param jobid jobid of the uint64_t type.
Expand All @@ -137,14 +135,11 @@ func ReapiCliMatchAllocate(
//
// const uint64_t jobid, const char *R, int64_t *at,
// double *ov, const char **R_out);
func ReapiCliUpdateAllocate(ctx *ReapiCtx, jobid int, r string) (at int64, overhead float64, r_out string, err error) {
func (cli *ReapiClient) UpdateAllocate(jobid int, r string) (at int64, overhead float64, r_out string, err error) {
var tmp_rout = C.CString("")
defer C.free(unsafe.Pointer(tmp_rout))

var resource = C.CString(r)
defer C.free(unsafe.Pointer(resource))

fluxerr := (int)(C.reapi_cli_update_allocate((*C.struct_reapi_cli_ctx)(ctx),
fluxerr := (int)(C.reapi_cli_update_allocate((*C.struct_reapi_cli_ctx)(cli.ctx),
(C.ulong)(jobid),
resource,
(*C.long)(&at),
Expand All @@ -157,7 +152,7 @@ func ReapiCliUpdateAllocate(ctx *ReapiCtx, jobid int, r string) (at int64, overh
return at, overhead, r_out, err
}

// ReapiCliCancel cancels the allocation or reservation corresponding to jobid.
// Cancel cancels the allocation or reservation corresponding to jobid.
//
// \param ctx reapi_cli_ctx_t context object
// \param jobid jobid of the uint64_t type.
Expand All @@ -167,14 +162,14 @@ func ReapiCliUpdateAllocate(ctx *ReapiCtx, jobid int, r string) (at int64, overh
// int reapi_cli_cancel (reapi_cli_ctx_t *ctx,
//
// const uint64_t jobid, bool noent_ok);
func ReapiCliCancel(ctx *ReapiCtx, jobid int64, noent_ok bool) (err error) {
fluxerr := (int)(C.reapi_cli_cancel((*C.struct_reapi_cli_ctx)(ctx),
func (cli *ReapiClient) Cancel(jobid int64, noent_ok bool) (err error) {
fluxerr := (int)(C.reapi_cli_cancel((*C.struct_reapi_cli_ctx)(cli.ctx),
(C.ulong)(jobid),
(C.bool)(noent_ok)))
return retvalToError(fluxerr, "issue resource api client cancel")
}

// ReapiCliInfo gets the information on the allocation or reservation corresponding
// Info gets the information on the allocation or reservation corresponding
//
// to jobid.
// \param ctx reapi_cli_ctx_t context object
Expand All @@ -191,11 +186,10 @@ func ReapiCliCancel(ctx *ReapiCtx, jobid int64, noent_ok bool) (err error) {
// int reapi_cli_info (reapi_cli_ctx_t *ctx, const uint64_t jobid,
//
// bool *reserved, int64_t *at, double *ov);
func ReapiCliInfo(ctx *ReapiCtx, jobid int64) (reserved bool, at int64, overhead float64, mode string, err error) {
func (cli *ReapiClient) Info(jobid int64) (reserved bool, at int64, overhead float64, mode string, err error) {
var tmp_mode = C.CString("")
defer C.free(unsafe.Pointer(tmp_mode))

fluxerr := (int)(C.reapi_cli_info((*C.struct_reapi_cli_ctx)(ctx),
fluxerr := (int)(C.reapi_cli_info((*C.struct_reapi_cli_ctx)(cli.ctx),
(C.ulong)(jobid),
(&tmp_mode),
(*C.bool)(&reserved),
Expand All @@ -206,7 +200,7 @@ func ReapiCliInfo(ctx *ReapiCtx, jobid int64) (reserved bool, at int64, overhead
return reserved, at, overhead, C.GoString(tmp_mode), err
}

// ReapiCliStat gets the performance information about the resource infrastructure.
// Stat gets the performance information about the resource infrastructure.
//
// \param ctx reapi_cli_ctx_t context object
// \param V Number of resource vertices
Expand All @@ -222,9 +216,9 @@ func ReapiCliInfo(ctx *ReapiCtx, jobid int64) (reserved bool, at int64, overhead
//
// int64_t *J, double *load,
// double *min, double *max, double *avg);
func ReapiCliStat(ctx *ReapiCtx) (v int64, e int64,
func (cli *ReapiClient) Stat() (v int64, e int64,
jobs int64, load float64, min float64, max float64, avg float64, err error) {
fluxerr := (int)(C.reapi_cli_stat((*C.struct_reapi_cli_ctx)(ctx),
fluxerr := (int)(C.reapi_cli_stat((*C.struct_reapi_cli_ctx)(cli.ctx),
(*C.long)(&v),
(*C.long)(&e),
(*C.long)(&jobs),
Expand All @@ -237,18 +231,18 @@ func ReapiCliStat(ctx *ReapiCtx) (v int64, e int64,
return v, e, jobs, load, min, max, avg, err
}

// ReapiCliGetErrMsg returns a string error message from the resource api
func ReapiCliGetErrMsg(ctx *ReapiCtx) string {
errmsg := C.reapi_cli_get_err_msg((*C.struct_reapi_cli_ctx)(ctx))
// GetErrMsg returns a string error message from the resource api
func (cli *ReapiClient) GetErrMsg() string {
errmsg := C.reapi_cli_get_err_msg((*C.struct_reapi_cli_ctx)(cli.ctx))
return C.GoString(errmsg)
}

// ReapiCliGetErrMsg clears error messages
func ReapiCliClearErrMsg(ctx *ReapiCtx) {
C.reapi_cli_clear_err_msg((*C.struct_reapi_cli_ctx)(ctx))
// ClearErrMsg clears error messages
func (cli *ReapiClient) ClearErrMsg() {
C.reapi_cli_clear_err_msg((*C.struct_reapi_cli_ctx)(cli.ctx))
}

// ReapiCliSetHandleSet emulates setting the opaque handle to the reapi cli context.
// SetHandleSet emulates setting the opaque handle to the reapi cli context.
// \param ctx reapi_cli_ctx_t context object
// \param h Opaque handle. How it is used is an implementation
//
Expand All @@ -258,16 +252,16 @@ func ReapiCliClearErrMsg(ctx *ReapiCtx) {
// \return 0 on success; -1 on error.
//
// int reapi_cli_set_handle (reapi_cli_ctx_t *ctx, void *handle);
func ReapiCliSetHandle(ctx *ReapiCtx) int {
func SetHandle() int {
return -1
}

// ReapiCliGetHandle emulates setting the opaque handle to the reapi cli context.
// GetHandle emulates setting the opaque handle to the reapi cli context.
//
// \param ctx reapi_cli_ctx_t context object
// \return handle
//
// void *reapi_cli_get_handle (reapi_cli_ctx_t *ctx);
func ReapiCliGetHandle(ctx *ReapiCtx) int {
func GetHandle() int {
return -1
}
4 changes: 2 additions & 2 deletions resource/reapi/bindings/go/src/fluxcli/reapi_cli_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ package fluxcli
import "testing"

func TestFluxcli(t *testing.T) {
ctx := NewReapiCli()
if ctx == nil {
cli := NewReapiClient()
if !cli.HasContext() {
t.Errorf("Context is null")
}

Expand Down
44 changes: 25 additions & 19 deletions resource/reapi/bindings/go/src/fluxmodule/reapi_module.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,22 @@ import (
)

type (
ReapiCtx C.struct_reapi_module_ctx_t
ReapiCtx C.struct_reapi_module_ctx_t
ReapiModule struct {
ctx ReapiCtx
}
)

// NewReapiModule creates a new resource API module
// reapi_module_ctx_t *reapi_module_new ();
func NewReapiModule() *ReapiCtx {
return (*ReapiCtx)(C.reapi_module_new())
func NewReapiModule() *ReapiModule {
ctx := (*ReapiCtx)(C.reapi_module_new())
return &ReapiModule{ctx: ctx}
}

// HasContext exposes the private ctx, telling the caller if it is set
func (m *ReapiModule) HasContext() bool {
return m.ctx != nil
}

// Given an integer return code, convert to go error
Expand All @@ -36,30 +45,27 @@ func retvalToError(code int, message string) error {
return fmt.Errorf(message+" %d", code)
}

// ReapiModuleDestroy destroys the resource API context
// Destroy destroys the resource API context
// void reapi_module_destroy (reapi_module_ctx_t *ctx);
func ReapiModuleDestroy(ctx *ReapiCtx) {
C.reapi_module_destroy((*C.struct_reapi_module_ctx)(ctx))
func (m *ReapiModule) Destroy() {
C.reapi_module_destroy((*C.struct_reapi_module_ctx)(m.ctx))
}

// ReapiModuleMatchAllocate matches and allocates resources
// MatchAllocate matches and allocates resources
// int reapi_module_match_allocate (reapi_module_ctx_t *ctx, bool orelse_reserve,
// at: is the scheduled time "at"
func ReapiModuleMatchAllocate(
ctx *ReapiCtx,
func (m *ReapiModule) MatchAllocate(
orelse_reserve bool,
jobspec string,
jobid int,
) (reserved bool, allocated string, at int64, overhead float64, err error) {
// var atlong C.long = (C.long)(at)
var r = C.CString("teststring")
defer C.free(unsafe.Pointer(r))

// Jobspec as a CString
spec := C.CString(jobspec)
defer C.free(unsafe.Pointer(spec))

fluxerr := (int)(C.reapi_module_match_allocate((*C.struct_reapi_module_ctx)(ctx),
fluxerr := (int)(C.reapi_module_match_allocate((*C.struct_reapi_module_ctx)(m.ctx),
(C.bool)(orelse_reserve),
spec,
(C.ulong)(jobid),
Expand All @@ -74,7 +80,7 @@ func ReapiModuleMatchAllocate(
return reserved, allocated, at, overhead, err
}

// ReapiModuleInfo gets the information on the allocation or reservation corresponding
// Info gets the information on the allocation or reservation corresponding
// to jobid.
//
// \param ctx reapi_module_ctx_t context object
Expand All @@ -91,8 +97,8 @@ func ReapiModuleMatchAllocate(
// int reapi_module_info (reapi_module_ctx_t *ctx, const uint64_t jobid,
//
// bool *reserved, int64_t *at, double *ov);
func ReapiModuleInfo(ctx *ReapiCtx, jobid int64) (reserved bool, at int64, overhead float64, err error) {
fluxerr := (int)(C.reapi_module_info((*C.struct_reapi_module_ctx)(ctx),
func (m *ReapiModule) Info(ctx *ReapiCtx, jobid int64) (reserved bool, at int64, overhead float64, err error) {
fluxerr := (int)(C.reapi_module_info((*C.struct_reapi_module_ctx)(m.ctx),
(C.ulong)(jobid),
(*C.bool)(&reserved),
(*C.long)(&at),
Expand All @@ -102,7 +108,7 @@ func ReapiModuleInfo(ctx *ReapiCtx, jobid int64) (reserved bool, at int64, overh
return reserved, at, overhead, err
}

// ReapiModuleCancel cancels the allocation or reservation corresponding to jobid.
// Cancel cancels the allocation or reservation corresponding to jobid.
//
// \param ctx reapi_module_ctx_t context object
// \param jobid jobid of the uint64_t type.
Expand All @@ -112,9 +118,9 @@ func ReapiModuleInfo(ctx *ReapiCtx, jobid int64) (reserved bool, at int64, overh
// int reapi_module_cancel (reapi_module_ctx_t *ctx,
//
// const uint64_t jobid, bool noent_ok);*/
func ReapiModuleCancel(ctx *ReapiCtx, jobid int64, noent_ok bool) (err error) {
fluxerr := (int)(C.reapi_module_cancel((*C.struct_reapi_module_ctx)(ctx),
func (m *ReapiModule) Cancel(jobid int64, noent_ok bool) (err error) {
fluxerr := (int)(C.reapi_module_cancel((*C.struct_reapi_module_ctx)(m.ctx),
(C.ulong)(jobid),
(C.bool)(noent_ok)))
return retvalToError(fluxerr, "issue with cancel")
}
}
10 changes: 5 additions & 5 deletions resource/reapi/bindings/go/src/fluxmodule/reapi_module_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,17 @@ package fluxmodule

import (
"fmt"
"io/ioutil"
"os"
"testing"
)

func TestFluxmodule(t *testing.T) {
ctx := NewReapiModule()
jobspec, err := ioutil.ReadFile("/root/flux-sched/t/data/resource/jobspecs/basics/test001.yaml")
if ctx == nil {
mod := NewReapiModule()
jobspec, err := os.ReadFile("/root/flux-sched/t/data/resource/jobspecs/basics/test001.yaml")
if !mod.HasContext() {
t.Errorf("Context is null")
}
reserved, allocated, at, overhead, err1 := ReapiModuleMatchAllocate(ctx, false, string(jobspec), 4)
reserved, allocated, at, overhead, err1 := mod.MatchAllocate(false, string(jobspec), 4)
fmt.Printf("%t, %s, %d, %f, %d, %s", reserved, allocated, at, overhead, err1, err)

}
Loading

0 comments on commit 9089091

Please sign in to comment.