Skip to content

fpga: make admission webhook mode-less #358

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
May 5, 2020
Merged

fpga: make admission webhook mode-less #358

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6c2eacf
webhook: remove mode of operation
Apr 2, 2020
99fcb69
fpga: compress fpga AF resource names
Apr 24, 2020
8d6afae
fpga: update e2e tests to work with mode-less webhook
bart0sh Apr 27, 2020
c63dbf6
fpgawebhook: move to v2 API of fpga.intel.com group
May 4, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion DEVEL.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ https://github.com/kubernetes/code-generator/blob/master/generate-groups.sh
```
$ generate-groups.sh all github.com/intel/intel-device-plugins-for-kubernetes/pkg/client \
github.com/intel/intel-device-plugins-for-kubernetes/pkg/apis \
fpga.intel.com:v1
fpga.intel.com:v2
```

Please note that the script (at least of v0.18.2-beta.0) expects the device plugins
Expand Down
58 changes: 34 additions & 24 deletions cmd/fpga_admissionwebhook/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,16 +30,6 @@ The admission controller also keeps the user from bypassing namespaced mapping r
by denying admission of any pods that are trying to use internal knowledge of InterfaceID or
Bitstream ID environment variables used by the prestart hook.

The admission controller can operate in two separate modes - preprogrammed or orchestration programmed.
The mode must be chosen to match that of the [FPGA plugin](../fpga_plugin/README.md) configuraton, as
shown in the following table:

| FPGA plugin mode | matching admission controller mode |
|:---------------- |:---------------------------------- |
| region | orchestrated |
| af | preprogrammed |


# Dependencies

This component is one of a set of components that work together. You may also want to
Expand Down Expand Up @@ -118,14 +108,6 @@ Register webhook
mutatingwebhookconfiguration.admissionregistration.k8s.io/fpga-mutator-webhook-cfg created
```

By default, the script deploys the webhook in a preprogrammed mode.

Use the option `--mode` script option to deploy the webhook in orchestrated mode:

```bash
$ ./scripts/webhook-deploy.sh --mode orchestrated
```

The script needs the CA bundle used for signing certificate requests in your cluster.
By default, the script fetches the bundle stored in the configmap
`extension-apiserver-authentication`. However, your cluster may use a different signing
Expand All @@ -138,13 +120,41 @@ $ ./scripts/webhook-deploy.sh --ca-bundle-path /var/run/kubernetes/server-ca.crt

# Mappings

Requested FPGA resources are translated to AF resources. For example,
`fpga.intel.com/arria10.dcp1.1-nlb0` is translated to `fpga.intel.com/af-d8424dc4a4a3c413f89e433683f9040b`.
Mappings is a an essential part of the setup that gives a flexible instrument to a cluster
administrator to manage FPGA bitstreams and to control access to them. Being a set of
custom resource definitions they are used to configure the way FPGA resource requests get
translated into actual resources provided by the cluster.

For the following mapping
Copy link
Member

Choose a reason for hiding this comment

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

It might be good to say a sentence or two, kind of "introduction", that mappings are essential part of the setup and give cluster administrator flexible instrument for managing FPGA bitstreams and access control for them?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! added


```yaml
apiVersion: fpga.intel.com/v2
kind: AcceleratorFunction
metadata:
name: arria10.dcp1.2-nlb0-preprogrammed
spec:
afuId: d8424dc4a4a3c413f89e433683f9040b
interfaceId: 69528db6eb31577a8c3668f9faa081f6
mode: af
```

In orchestrated mode, `fpga.intel.com/arria10.dcp1.1-nlb0` gets translated to
`fpga.intel.com/region-9926ab6d6c925a68aabca7d84c545738`, and, the corresponding AF IDs are set in
environment variables for the container. The [FPGA CRI-O hook](../fpga_crihook/README.md)
then loads the requested bitstream to a region before the container is started.
requested FPGA resources are translated to AF resources. For example,
`fpga.intel.com/arria10.dcp1.2-nlb0-preprogrammed` is translated to
`fpga.intel.com/af-695.d84.aVKNtusxV3qMNmj5-qCB9thCTcSko8QT-J5DNoP5BAs` where the `af-`
prefix indicates the plugin's mode (`af`), `695` is the first three characters of
the region interface ID, `d84` is the first three characters of the accelerator function ID
and the last part `aVKNtusxV3qMNmj5-qCB9thCTcSko8QT-J5DNoP5BAs` is a base64-encoded concatenation
of the full region interface ID and accelerator function ID.
The format of resource names (e.g. `arria10.dcp1.2-nlb0-preprogrammed`) can be any and is up
to a cluster administrator.

The same mapping, but with its mode field set to `region`, would translate
`fpga.intel.com/arria10.dcp1.2-nlb0-preprogrammed` to `fpga.intel.com/region-69528db6eb31577a8c3668f9faa081f6`,
and the corresponding AF IDs are set in environment variables for the container.
Though in this case the cluster administrator would probably want to rename
the mapping `arria10.dcp1.2-nlb0-preprogrammed` to something like `arria10.dcp1.2-nlb0-orchestrated`
to reflect its mode. The [FPGA CRI-O hook](../fpga_crihook/README.md) then loads the requested
bitstream to a region before the container is started.

Mappings of resource names are configured with objects of `AcceleratorFunction` and
`FpgaRegion` custom resource definitions found respectively in
Expand Down
25 changes: 8 additions & 17 deletions cmd/fpga_admissionwebhook/controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import (

clientset "github.com/intel/intel-device-plugins-for-kubernetes/pkg/client/clientset/versioned"
informers "github.com/intel/intel-device-plugins-for-kubernetes/pkg/client/informers/externalversions"
listers "github.com/intel/intel-device-plugins-for-kubernetes/pkg/client/listers/fpga.intel.com/v1"
listers "github.com/intel/intel-device-plugins-for-kubernetes/pkg/client/listers/fpga.intel.com/v2"
)

const (
Expand All @@ -42,7 +42,7 @@ type fpgaObjectKey struct {
}

type controller struct {
patcherManager *patcherManager
patcherManager patcherManager
informerFactory informers.SharedInformerFactory
afsSynced cache.InformerSynced
regionsSynced cache.InformerSynced
Expand All @@ -52,7 +52,7 @@ type controller struct {
stopCh chan struct{}
}

func newController(patcherManager *patcherManager, config *rest.Config) (*controller, error) {
func newController(patcherManager patcherManager, config *rest.Config) (*controller, error) {
clientset, err := clientset.NewForConfig(config)
if err != nil {
return nil, errors.Wrap(err, "Failed to create REST clientset")
Expand All @@ -61,8 +61,8 @@ func newController(patcherManager *patcherManager, config *rest.Config) (*contro
informerFactory := informers.NewSharedInformerFactory(clientset, resyncPeriod)
stopCh := make(chan struct{})

afInformer := informerFactory.Fpga().V1().AcceleratorFunctions()
regionInformer := informerFactory.Fpga().V1().FpgaRegions()
afInformer := informerFactory.Fpga().V2().AcceleratorFunctions()
regionInformer := informerFactory.Fpga().V2().FpgaRegions()

controller := &controller{
patcherManager: patcherManager,
Expand Down Expand Up @@ -172,11 +172,7 @@ func (c *controller) syncAfHandler(key string) error {
return nil
}

patcher, err := c.patcherManager.getPatcher(namespace)
if err != nil {
runtime.HandleError(errors.Wrapf(err, "can't get patcher for namespace %s", namespace))
return nil
}
patcher := c.patcherManager.getPatcher(namespace)

// Get the AcceleratorFunction resource with this namespace/name
af, err := c.afLister.AcceleratorFunctions(namespace).Get(name)
Expand All @@ -194,8 +190,7 @@ func (c *controller) syncAfHandler(key string) error {
}

klog.V(4).Info("Received", af)
patcher.addAf(af)
return nil
return patcher.addAf(af)
}

func (c *controller) syncRegionHandler(key string) error {
Expand All @@ -206,11 +201,7 @@ func (c *controller) syncRegionHandler(key string) error {
return nil
}

patcher, err := c.patcherManager.getPatcher(namespace)
if err != nil {
runtime.HandleError(errors.Wrapf(err, "can't get patcher for namespace %s", namespace))
return nil
}
patcher := c.patcherManager.getPatcher(namespace)

// Get the FpgaRegion resource with this namespace/name
region, err := c.regionLister.FpgaRegions(namespace).Get(name)
Expand Down
88 changes: 32 additions & 56 deletions cmd/fpga_admissionwebhook/controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,29 +24,29 @@ import (
"k8s.io/apimachinery/pkg/labels"
"k8s.io/client-go/rest"

v1 "github.com/intel/intel-device-plugins-for-kubernetes/pkg/apis/fpga.intel.com/v1"
listers "github.com/intel/intel-device-plugins-for-kubernetes/pkg/client/listers/fpga.intel.com/v1"
v2 "github.com/intel/intel-device-plugins-for-kubernetes/pkg/apis/fpga.intel.com/v2"
listers "github.com/intel/intel-device-plugins-for-kubernetes/pkg/client/listers/fpga.intel.com/v2"
)

type fakeAfNamespaceLister struct {
af *v1.AcceleratorFunction
af *v2.AcceleratorFunction
err error
}

func init() {
flag.Set("v", "4") ///Enable debug output
}

func (nl *fakeAfNamespaceLister) Get(name string) (*v1.AcceleratorFunction, error) {
func (nl *fakeAfNamespaceLister) Get(name string) (*v2.AcceleratorFunction, error) {
return nl.af, nl.err
}

func (nl *fakeAfNamespaceLister) List(selector labels.Selector) (ret []*v1.AcceleratorFunction, err error) {
func (nl *fakeAfNamespaceLister) List(selector labels.Selector) (ret []*v2.AcceleratorFunction, err error) {
return nil, nil
}

type fakeAfLister struct {
af *v1.AcceleratorFunction
af *v2.AcceleratorFunction
err error
}

Expand All @@ -57,17 +57,16 @@ func (l *fakeAfLister) AcceleratorFunctions(namespace string) listers.Accelerato
}
}

func (l *fakeAfLister) List(selector labels.Selector) (ret []*v1.AcceleratorFunction, err error) {
func (l *fakeAfLister) List(selector labels.Selector) (ret []*v2.AcceleratorFunction, err error) {
return nil, nil
}

func TestSyncAfHandler(t *testing.T) {
tcases := []struct {
name string
key string
afLister *fakeAfLister
patcherManagerIsBroken bool
expectedErr bool
name string
key string
afLister *fakeAfLister
expectedErr bool
}{
{
name: "Wrong key format",
Expand All @@ -77,21 +76,16 @@ func TestSyncAfHandler(t *testing.T) {
name: "Known key",
key: "default/arria10-nlb0",
afLister: &fakeAfLister{
af: &v1.AcceleratorFunction{
af: &v2.AcceleratorFunction{
ObjectMeta: metav1.ObjectMeta{
Name: "arria10-nlb0",
},
Spec: v1.AcceleratorFunctionSpec{
Spec: v2.AcceleratorFunctionSpec{
AfuID: "d8424dc4a4a3c413f89e433683f9040b",
},
},
},
},
{
name: "Broken patcher manager",
key: "default/arria10-nlb0",
patcherManagerIsBroken: true,
},
{
name: "Unknown key",
key: "default/unknown",
Expand All @@ -107,13 +101,7 @@ func TestSyncAfHandler(t *testing.T) {
}

for _, tt := range tcases {
pm, err := newPatcherManager(preprogrammed)
if err != nil {
t.Fatalf("Test case '%s': %+v", tt.name, err)
}
if tt.patcherManagerIsBroken {
pm.defaultMode = "broken"
}
pm := newPatcherManager()
c, err := newController(pm, &rest.Config{})
if err != nil {
t.Fatalf("Test case '%s': %+v", tt.name, err)
Expand All @@ -132,20 +120,20 @@ func TestSyncAfHandler(t *testing.T) {
}

type fakeRegionNamespaceLister struct {
region *v1.FpgaRegion
region *v2.FpgaRegion
err error
}

func (nl *fakeRegionNamespaceLister) Get(name string) (*v1.FpgaRegion, error) {
func (nl *fakeRegionNamespaceLister) Get(name string) (*v2.FpgaRegion, error) {
return nl.region, nl.err
}

func (nl *fakeRegionNamespaceLister) List(selector labels.Selector) (ret []*v1.FpgaRegion, err error) {
func (nl *fakeRegionNamespaceLister) List(selector labels.Selector) (ret []*v2.FpgaRegion, err error) {
return nil, nil
}

type fakeRegionLister struct {
region *v1.FpgaRegion
region *v2.FpgaRegion
err error
}

Expand All @@ -156,17 +144,16 @@ func (l *fakeRegionLister) FpgaRegions(namespace string) listers.FpgaRegionNames
}
}

func (l *fakeRegionLister) List(selector labels.Selector) (ret []*v1.FpgaRegion, err error) {
func (l *fakeRegionLister) List(selector labels.Selector) (ret []*v2.FpgaRegion, err error) {
return nil, nil
}

func TestSyncRegionHandler(t *testing.T) {
tcases := []struct {
name string
key string
patcherManagerIsBroken bool
regionLister *fakeRegionLister
expectedErr bool
name string
key string
regionLister *fakeRegionLister
expectedErr bool
}{
{
name: "Wrong key format",
Expand All @@ -176,21 +163,16 @@ func TestSyncRegionHandler(t *testing.T) {
name: "Known key",
key: "default/arria10",
regionLister: &fakeRegionLister{
region: &v1.FpgaRegion{
region: &v2.FpgaRegion{
ObjectMeta: metav1.ObjectMeta{
Name: "arria10",
},
Spec: v1.FpgaRegionSpec{
Spec: v2.FpgaRegionSpec{
InterfaceID: "ce48969398f05f33946d560708be108a",
},
},
},
},
{
name: "Broken patcher manager",
key: "default/arria10",
patcherManagerIsBroken: true,
},
{
name: "Unknown key",
key: "default/unknown",
Expand All @@ -206,13 +188,7 @@ func TestSyncRegionHandler(t *testing.T) {
}

for _, tt := range tcases {
pm, err := newPatcherManager(preprogrammed)
if err != nil {
t.Fatalf("Test case '%s': %+v", tt.name, err)
}
if tt.patcherManagerIsBroken {
pm.defaultMode = "broken"
}
pm := newPatcherManager()
c, err := newController(pm, &rest.Config{})
if err != nil {
t.Fatalf("Test case '%s': %+v", tt.name, err)
Expand Down Expand Up @@ -328,7 +304,7 @@ func TestProcessNextWorkItem(t *testing.T) {
},
}
for _, tt := range tcases {
pm, _ := newPatcherManager(preprogrammed)
pm := newPatcherManager()
c, err := newController(pm, &rest.Config{})
if err != nil {
t.Fatalf("Test case '%s': %+v", tt.name, err)
Expand All @@ -352,9 +328,9 @@ func TestProcessNextWorkItem(t *testing.T) {

func TestCreateEventhandler(t *testing.T) {
funcs := createEventHandler("testkind", &fakeQueue{})
funcs.AddFunc(&v1.FpgaRegion{})
funcs.UpdateFunc(nil, &v1.FpgaRegion{})
funcs.DeleteFunc(&v1.FpgaRegion{})
funcs.AddFunc(&v2.FpgaRegion{})
funcs.UpdateFunc(nil, &v2.FpgaRegion{})
funcs.DeleteFunc(&v2.FpgaRegion{})
}

func TestRun(t *testing.T) {
Expand All @@ -369,7 +345,7 @@ func TestRun(t *testing.T) {
}

for _, tt := range tcases {
pm := &patcherManager{}
pm := newPatcherManager()
c, err := newController(pm, &rest.Config{})
if err != nil {
t.Fatalf("Test case '%s': %+v", tt.name, err)
Expand Down Expand Up @@ -404,7 +380,7 @@ func TestNewController(t *testing.T) {
config := &rest.Config{
Host: tt.configHost,
}
pm := &patcherManager{}
pm := newPatcherManager()
c, err := newController(pm, config)
if err != nil && !tt.expectedErr {
t.Errorf("Test case '%s': unexpected error: %+v", tt.name, err)
Expand Down
Loading