Skip to content

Commit

Permalink
Credentials support for S3 and GCS storage (kubeflow#94)
Browse files Browse the repository at this point in the history
* Reconcile service account

* Add service account/secret role binding

* Add GCS secret volume

* Add tests for credential reconciliation

* Reorganize credential files and rename reconciler to builder
  • Loading branch information
yuzisun authored and k8s-ci-robot committed May 21, 2019
1 parent 99af0dd commit 4f4022c
Show file tree
Hide file tree
Showing 15 changed files with 794 additions and 48 deletions.
12 changes: 12 additions & 0 deletions config/default/crds/serving_v1alpha1_kfservice.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,12 @@ spec:
- container
type: object
maxReplicas:
description: This is the up bound for autoscaler to scale to
format: int64
type: integer
minReplicas:
description: Minimum number of replicas, pods won't scale down to
0 in case of no traffic
format: int64
type: integer
scikitLearn:
Expand All @@ -59,6 +62,9 @@ spec:
required:
- modelUri
type: object
serviceAccountName:
description: Service Account Name
type: string
tensorflow:
properties:
modelUri:
Expand Down Expand Up @@ -103,9 +109,12 @@ spec:
- container
type: object
maxReplicas:
description: This is the up bound for autoscaler to scale to
format: int64
type: integer
minReplicas:
description: Minimum number of replicas, pods won't scale down to
0 in case of no traffic
format: int64
type: integer
scikitLearn:
Expand All @@ -121,6 +130,9 @@ spec:
required:
- modelUri
type: object
serviceAccountName:
description: Service Account Name
type: string
tensorflow:
properties:
modelUri:
Expand Down
16 changes: 16 additions & 0 deletions config/default/rbac/rbac_role.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,22 @@ rules:
- get
- update
- patch
- apiGroups:
- ""
resources:
- serviceaccounts
verbs:
- get
- list
- watch
- apiGroups:
- ""
resources:
- secrets
verbs:
- get
- list
- watch
- apiGroups:
- admissionregistration.k8s.io
resources:
Expand Down
4 changes: 4 additions & 0 deletions pkg/apis/serving/v1alpha1/kfservice_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,11 @@ type KFServiceSpec struct {

// ModelSpec defines the default configuration to route traffic.
type ModelSpec struct {
// Service Account Name
ServiceAccountName string `json:"serviceAccountName,omitempty"`
// Minimum number of replicas, pods won't scale down to 0 in case of no traffic
MinReplicas int `json:"minReplicas,omitempty"`
// This is the up bound for autoscaler to scale to
MaxReplicas int `json:"maxReplicas,omitempty"`
// The following fields follow a "1-of" semantic. Users must specify exactly one spec.
Custom *CustomSpec `json:"custom,omitempty"`
Expand Down
23 changes: 19 additions & 4 deletions pkg/controller/kfservice/kfservice_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,11 @@ package service
import (
"context"
"github.com/kubeflow/kfserving/pkg/constants"

"k8s.io/apimachinery/pkg/types"

"github.com/kubeflow/kfserving/pkg/reconciler/ksvc"
"github.com/kubeflow/kfserving/pkg/reconciler/ksvc/resources"
v1 "k8s.io/api/core/v1"
"k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/equality"
"k8s.io/client-go/tools/record"

Expand Down Expand Up @@ -119,6 +118,8 @@ type ReconcileService struct {
// +kubebuilder:rbac:groups=serving.knative.dev,resources=routes/status,verbs=get;update;patch
// +kubebuilder:rbac:groups=serving.kubeflow.org,resources=kfservices,verbs=get;list;watch;create;update;patch;delete
// +kubebuilder:rbac:groups=serving.kubeflow.org,resources=kfservices/status,verbs=get;update;patch
// +kubebuilder:rbac:groups=,resources=serviceaccounts,verbs=get;list;watch
// +kubebuilder:rbac:groups=,resources=secrets,verbs=get;list;watch
func (r *ReconcileService) Reconcile(request reconcile.Request) (reconcile.Result, error) {
// Fetch the KFService instance
kfsvc := &kfservingv1alpha1.KFService{}
Expand All @@ -132,16 +133,24 @@ func (r *ReconcileService) Reconcile(request reconcile.Request) (reconcile.Resul
return reconcile.Result{}, err
}

credentialBuilder := ksvc.NewCredentialBulder(r.Client)

serviceReconciler := ksvc.NewServiceReconciler(r.Client)
// Reconcile configurations

desiredDefault := resources.CreateKnativeConfiguration(constants.DefaultConfigurationName(kfsvc.Name),
kfsvc.ObjectMeta, &kfsvc.Spec.Default)

if err := controllerutil.SetControllerReference(kfsvc, desiredDefault, r.scheme); err != nil {
return reconcile.Result{}, err
}

defaultConfiguration, err := serviceReconciler.ReconcileConfiguarion(context.TODO(), desiredDefault)
if err := credentialBuilder.CreateSecretVolumeAndEnv(context.TODO(), request.Namespace, kfsvc.Spec.Default.ServiceAccountName,
desiredDefault); err != nil {
log.Error(err, "Failed to create credential volume or envs", "ServiceAccount", kfsvc.Spec.Default.ServiceAccountName)
}

defaultConfiguration, err := serviceReconciler.ReconcileConfiguration(context.TODO(), desiredDefault)
if err != nil {
log.Error(err, "Failed to reconcile default model spec", "name", desiredDefault.Name)
r.Recorder.Eventf(kfsvc, v1.EventTypeWarning, "InternalError", err.Error())
Expand All @@ -156,7 +165,13 @@ func (r *ReconcileService) Reconcile(request reconcile.Request) (reconcile.Resul
if err := controllerutil.SetControllerReference(kfsvc, desiredCanary, r.scheme); err != nil {
return reconcile.Result{}, err
}
canaryConfiguration, err := serviceReconciler.ReconcileConfiguarion(context.TODO(), desiredCanary)

if err := credentialBuilder.CreateSecretVolumeAndEnv(context.TODO(), request.Namespace, kfsvc.Spec.Canary.ServiceAccountName,
desiredCanary); err != nil {
log.Error(err, "Failed to create credential volume or envs", "ServiceAccount", kfsvc.Spec.Canary.ServiceAccountName)
}

canaryConfiguration, err := serviceReconciler.ReconcileConfiguration(context.TODO(), desiredCanary)
if err != nil {
log.Error(err, "Failed to reconcile canary model spec", "name", desiredCanary.Name)
r.Recorder.Eventf(kfsvc, v1.EventTypeWarning, "InternalError", err.Error())
Expand Down
30 changes: 7 additions & 23 deletions pkg/controller/kfservice/kfservice_controller_suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,38 +17,22 @@ limitations under the License.
package service

import (
knservingv1alpha1 "github.com/knative/serving/pkg/apis/serving/v1alpha1"
stdlog "log"
"os"
"path/filepath"
"sync"
"testing"

"github.com/kubeflow/kfserving/pkg/apis"

pkgtest "github.com/kubeflow/kfserving/pkg/testing"
"github.com/onsi/gomega"
"k8s.io/client-go/kubernetes/scheme"
"k8s.io/client-go/rest"
"sigs.k8s.io/controller-runtime/pkg/envtest"
stdlog "log"
"os"
"sigs.k8s.io/controller-runtime/pkg/manager"
"sigs.k8s.io/controller-runtime/pkg/reconcile"
"sync"
"testing"
)

var cfg *rest.Config

func TestMain(m *testing.M) {
t := &envtest.Environment{
CRDDirectoryPaths: []string{filepath.Join("..", "..", "..", "config", "default", "crds"),
filepath.Join("..", "..", "..", "test", "crds")},
}
err := apis.AddToScheme(scheme.Scheme)
if err != nil {
log.Error(err, "failed to add to scheme")
}
err = knservingv1alpha1.SchemeBuilder.AddToScheme(scheme.Scheme)
if err != nil {
log.Error(err, "failed to add knative serving to scheme")
}
t := pkgtest.SetupEnvTest()
var err error
if cfg, err = t.Start(); err != nil {
stdlog.Fatal(err)
}
Expand Down
22 changes: 3 additions & 19 deletions pkg/reconciler/ksvc/ksvc_suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,36 +17,20 @@ limitations under the License.
package ksvc

import (
knservingv1alpha1 "github.com/knative/serving/pkg/apis/serving/v1alpha1"
"github.com/kubeflow/kfserving/pkg/apis/serving/v1alpha1"
pkgtest "github.com/kubeflow/kfserving/pkg/testing"
"k8s.io/client-go/kubernetes/scheme"
"k8s.io/client-go/rest"
"os"
"path/filepath"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/envtest"
"testing"
)

var cfg *rest.Config
var c client.Client

func TestMain(m *testing.M) {
t := &envtest.Environment{
CRDDirectoryPaths: []string{filepath.Join("..", "..", "..", "config", "default", "crds"),
filepath.Join("..", "..", "..", "test", "crds")},
}

err := v1alpha1.SchemeBuilder.AddToScheme(scheme.Scheme)

if err != nil {
log.Error(err, "Failed to add kfserving scheme")
}

if err = knservingv1alpha1.SchemeBuilder.AddToScheme(scheme.Scheme); err != nil {
log.Error(err, "Failed to add knative serving scheme")
}

t := pkgtest.SetupEnvTest()
var err error
if cfg, err = t.Start(); err != nil {
log.Error(err, "Failed to start testing panel")
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/reconciler/ksvc/reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func NewServiceReconciler(client client.Client) *ServiceReconciler {
// Reconcile compares the actual state with the desired, and attempts to
// converge the two. It then updates the Status block of the Service resource
// with the current status of the resource.
func (c *ServiceReconciler) ReconcileConfiguarion(ctx context.Context, desiredConfiguration *knservingv1alpha1.Configuration) (*knservingv1alpha1.Configuration, error) {
func (c *ServiceReconciler) ReconcileConfiguration(ctx context.Context, desiredConfiguration *knservingv1alpha1.Configuration) (*knservingv1alpha1.Configuration, error) {
configuration := &knservingv1alpha1.Configuration{}
err := c.client.Get(context.TODO(), types.NamespacedName{Name: desiredConfiguration.Name,
Namespace: desiredConfiguration.Namespace}, configuration)
Expand Down
2 changes: 1 addition & 1 deletion pkg/reconciler/ksvc/reconciler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ func TestKnativeConfigurationReconcile(t *testing.T) {
if scenario.update {
g.Expect(c.Create(context.TODO(), existingConfiguration)).NotTo(gomega.HaveOccurred())
}
configuration, err := serviceReconciler.ReconcileConfiguarion(context.TODO(), scenario.desiredConfiguration)
configuration, err := serviceReconciler.ReconcileConfiguration(context.TODO(), scenario.desiredConfiguration)
// Validate
if scenario.shouldFail && err == nil {
t.Errorf("Test %q failed: returned success but expected error", name)
Expand Down
45 changes: 45 additions & 0 deletions pkg/reconciler/ksvc/resources/credentials/gcs/gcs_secret.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
/*
Copyright 2019 kubeflow.org.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package gcs

import (
"k8s.io/api/core/v1"
)

const (
GCSCredentialFileName = "gcloud-application-credentials.json"
GCSCredentialVolumeName = "user-gcp-sa"
GCSCredentialVolumeMountPath = "/var/secrets/gcloud-application-credentials.json"
GCSCredentialEnvKey = "GOOGLE_APPLICATION_CREDENTIALS"
)

func BuildSecretVolume(secret *v1.Secret) (v1.Volume, v1.VolumeMount) {
volume := v1.Volume{
Name: GCSCredentialVolumeName,
VolumeSource: v1.VolumeSource{
Secret: &v1.SecretVolumeSource{
SecretName: secret.Name,
},
},
}
volumeMount := v1.VolumeMount{
MountPath: GCSCredentialVolumeMountPath,
Name: GCSCredentialVolumeName,
ReadOnly: true,
}
return volume, volumeMount
}
68 changes: 68 additions & 0 deletions pkg/reconciler/ksvc/resources/credentials/gcs/gcs_secret_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
/*
Copyright 2019 kubeflow.org.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package gcs

import (
"github.com/google/go-cmp/cmp"
"k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"testing"
)

func TestGcsSecret(t *testing.T) {
scenarios := map[string]struct {
secret *v1.Secret
expectedVolume v1.Volume
expectedVolumeMount v1.VolumeMount
}{
"GCSSecretVolume": {
secret: &v1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "user-gcp-sa",
},
Data: map[string][]byte{
GCSCredentialFileName: {},
},
},
expectedVolumeMount: v1.VolumeMount{
Name: GCSCredentialVolumeName,
ReadOnly: true,
MountPath: GCSCredentialVolumeMountPath,
},
expectedVolume: v1.Volume{
Name: GCSCredentialVolumeName,
VolumeSource: v1.VolumeSource{
Secret: &v1.SecretVolumeSource{
SecretName: "user-gcp-sa",
},
},
},
},
}

for name, scenario := range scenarios {
volume, volumeMount := BuildSecretVolume(scenario.secret)

if diff := cmp.Diff(scenario.expectedVolume, volume); diff != "" {
t.Errorf("Test %q unexpected volume (-want +got): %v", name, diff)
}

if diff := cmp.Diff(scenario.expectedVolumeMount, volumeMount); diff != "" {
t.Errorf("Test %q unexpected volumeMount (-want +got): %v", name, diff)
}
}
}
Loading

0 comments on commit 4f4022c

Please sign in to comment.