From d82186f74e12b72f38eb2b1bc00778e688d646cb Mon Sep 17 00:00:00 2001 From: Jing Xu Date: Wed, 2 Aug 2017 14:35:39 -0700 Subject: [PATCH 1/5] AttachDisk should not call detach inside of Cinder volume provider This PR fixes #50038 which removes the detach call inside of AttachDisk. --- .../providers/openstack/openstack_volumes.go | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/pkg/cloudprovider/providers/openstack/openstack_volumes.go b/pkg/cloudprovider/providers/openstack/openstack_volumes.go index 7f7b499bb7744..f9a46140c819d 100644 --- a/pkg/cloudprovider/providers/openstack/openstack_volumes.go +++ b/pkg/cloudprovider/providers/openstack/openstack_volumes.go @@ -230,11 +230,9 @@ func (os *OpenStack) AttachDisk(instanceID, volumeID string) (string, error) { glog.V(4).Infof("Disk %s is already attached to instance %s", volumeID, instanceID) return volume.ID, nil } - glog.V(2).Infof("Disk %s is attached to a different instance (%s), detaching", volumeID, volume.AttachedServerId) - err = os.DetachDisk(volume.AttachedServerId, volumeID) - if err != nil { - return "", err - } + errmsg := fmt.Sprintf("Disk %s is attached to a different instance (%s)", volumeID, volume.AttachedServerId) + glog.V(2).Infof(errmsg) + return "", errors.New(errmsg) } startTime := time.Now() From 87c8bf8f3953aedddda3a0764bfe0addd8a39108 Mon Sep 17 00:00:00 2001 From: Jesse Haka Date: Thu, 27 Jul 2017 12:36:25 +0300 Subject: [PATCH 2/5] add possibility to use multiple floating pools if not needed here load network ids from gophercloud api fix to getnetworkbyname update godeps, add networks library fix gofmt and boilerplate gofmt use annotations fix remove enableflag add comment to annotationvalue --- .../openstack/openstack_loadbalancer.go | 24 +++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/pkg/cloudprovider/providers/openstack/openstack_loadbalancer.go b/pkg/cloudprovider/providers/openstack/openstack_loadbalancer.go index 722063bfe16fa..ca085890009f6 100644 --- a/pkg/cloudprovider/providers/openstack/openstack_loadbalancer.go +++ b/pkg/cloudprovider/providers/openstack/openstack_loadbalancer.go @@ -66,6 +66,8 @@ const ( activeStatus = "ACTIVE" errorStatus = "ERROR" + + ServiceAnnotationLoadBalancerFloatingNetworkId = "loadbalancer.openstack.org/floating-network-id" ) // LoadBalancer implementation for LBaaS v1 @@ -581,6 +583,21 @@ func nodeAddressForLB(node *v1.Node) (string, error) { return addrs[0].Address, nil } +//getStringFromServiceAnnotation searches a given v1.Service for a specific annotationKey and either returns the annotation's value or a specified defaultSetting +func getStringFromServiceAnnotation(service *v1.Service, annotationKey string, defaultSetting string) string { + glog.V(4).Infof("getStringFromServiceAnnotation(%v, %v, %v)", service, annotationKey, defaultSetting) + if annotationValue, ok := service.Annotations[annotationKey]; ok { + //if there is an annotation for this setting, set the "setting" var to it + // annotationValue can be empty, it is working as designed + // it makes possible for instance provisioning loadbalancer without floatingip + glog.V(4).Infof("Found a Service Annotation: %v = %v", annotationKey, annotationValue) + return annotationValue + } + //if there is no annotation, set "settings" var to the value from cloud config + glog.V(4).Infof("Could not find a Service Annotation; falling back on cloud-config setting: %v = %v", annotationKey, defaultSetting) + return defaultSetting +} + // TODO: This code currently ignores 'region' and always creates a // loadbalancer in only the current OpenStack region. We should take // a list of regions (from config) and query/create loadbalancers in @@ -598,6 +615,9 @@ func (lbaas *LbaasV2) EnsureLoadBalancer(clusterName string, apiService *v1.Serv return nil, fmt.Errorf("no ports provided to openstack load balancer") } + floatingPool := getStringFromServiceAnnotation(apiService, ServiceAnnotationLoadBalancerFloatingNetworkId, lbaas.opts.FloatingNetworkId) + glog.V(4).Infof("EnsureLoadBalancer using floatingPool: %v", floatingPool) + // Check for TCP protocol on each port // TODO: Convert all error messages to use an event recorder for _, port := range ports { @@ -827,10 +847,10 @@ func (lbaas *LbaasV2) EnsureLoadBalancer(clusterName string, apiService *v1.Serv if err != nil && err != ErrNotFound { return nil, fmt.Errorf("Error getting floating ip for port %s: %v", portID, err) } - if floatIP == nil && lbaas.opts.FloatingNetworkId != "" { + if floatIP == nil && floatingPool != "" { glog.V(4).Infof("Creating floating ip for loadbalancer %s port %s", loadbalancer.ID, portID) floatIPOpts := floatingips.CreateOpts{ - FloatingNetworkID: lbaas.opts.FloatingNetworkId, + FloatingNetworkID: floatingPool, PortID: portID, } floatIP, err = floatingips.Create(lbaas.network, floatIPOpts).Extract() From 5f5e765bdbb9f2e8cfc2df83b35e6f7ddaadd0cf Mon Sep 17 00:00:00 2001 From: zhouhaibing089 Date: Mon, 9 Jan 2017 09:52:46 +0800 Subject: [PATCH 3/5] plugin/pkg/client/auth: add openstack auth provider --- .../plugin/pkg/client/auth/openstack/BUILD | 40 +++++ .../pkg/client/auth/openstack/openstack.go | 161 ++++++++++++++++++ .../client/auth/openstack/openstack_test.go | 116 +++++++++++++ 3 files changed, 317 insertions(+) create mode 100644 staging/src/k8s.io/client-go/plugin/pkg/client/auth/openstack/BUILD create mode 100644 staging/src/k8s.io/client-go/plugin/pkg/client/auth/openstack/openstack.go create mode 100644 staging/src/k8s.io/client-go/plugin/pkg/client/auth/openstack/openstack_test.go diff --git a/staging/src/k8s.io/client-go/plugin/pkg/client/auth/openstack/BUILD b/staging/src/k8s.io/client-go/plugin/pkg/client/auth/openstack/BUILD new file mode 100644 index 0000000000000..b7802cc6eb4c5 --- /dev/null +++ b/staging/src/k8s.io/client-go/plugin/pkg/client/auth/openstack/BUILD @@ -0,0 +1,40 @@ +package(default_visibility = ["//visibility:public"]) + +licenses(["notice"]) + +load( + "@io_bazel_rules_go//go:def.bzl", + "go_library", + "go_test", +) + +go_test( + name = "go_default_test", + srcs = ["openstack_test.go"], + library = ":go_default_library", + tags = ["automanaged"], +) + +go_library( + name = "go_default_library", + srcs = ["openstack.go"], + tags = ["automanaged"], + deps = [ + "//vendor/github.com/golang/glog:go_default_library", + "//vendor/github.com/gophercloud/gophercloud/openstack:go_default_library", + "//vendor/k8s.io/client-go/rest:go_default_library", + ], +) + +filegroup( + name = "package-srcs", + srcs = glob(["**"]), + tags = ["automanaged"], + visibility = ["//visibility:private"], +) + +filegroup( + name = "all-srcs", + srcs = [":package-srcs"], + tags = ["automanaged"], +) diff --git a/staging/src/k8s.io/client-go/plugin/pkg/client/auth/openstack/openstack.go b/staging/src/k8s.io/client-go/plugin/pkg/client/auth/openstack/openstack.go new file mode 100644 index 0000000000000..9df9491312209 --- /dev/null +++ b/staging/src/k8s.io/client-go/plugin/pkg/client/auth/openstack/openstack.go @@ -0,0 +1,161 @@ +/* +Copyright 2017 The Kubernetes Authors. + +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 openstack + +import ( + "fmt" + "net/http" + "sync" + "time" + + "github.com/golang/glog" + "github.com/gophercloud/gophercloud/openstack" + + restclient "k8s.io/client-go/rest" +) + +func init() { + if err := restclient.RegisterAuthProviderPlugin("openstack", newOpenstackAuthProvider); err != nil { + glog.Fatalf("Failed to register openstack auth plugin: %s", err) + } +} + +// DefaultTTLDuration is the time before a token gets expired. +const DefaultTTLDuration = 10 * time.Minute + +// openstackAuthProvider is an authprovider for openstack. this provider reads +// the environment variables to determine the client identity, and generates a +// token which will be inserted into the request header later. +type openstackAuthProvider struct { + ttl time.Duration + + tokenGetter TokenGetter +} + +// TokenGetter returns a bearer token that can be inserted into request. +type TokenGetter interface { + Token() (string, error) +} + +type tokenGetter struct{} + +// Token creates a token by authenticate with keystone. +func (*tokenGetter) Token() (string, error) { + options, err := openstack.AuthOptionsFromEnv() + if err != nil { + return "", fmt.Errorf("failed to read openstack env vars: %s", err) + } + client, err := openstack.AuthenticatedClient(options) + if err != nil { + return "", fmt.Errorf("authentication failed: %s", err) + } + return client.TokenID, nil +} + +// cachedGetter caches a token until it gets expired, after the expiration, it will +// generate another token and cache it. +type cachedGetter struct { + mutex sync.Mutex + tokenGetter TokenGetter + + token string + born time.Time + ttl time.Duration +} + +// Token returns the current available token, create a new one if expired. +func (c *cachedGetter) Token() (string, error) { + c.mutex.Lock() + defer c.mutex.Unlock() + + var err error + // no token or exceeds the TTL + if c.token == "" || time.Now().Sub(c.born) > c.ttl { + c.token, err = c.tokenGetter.Token() + if err != nil { + return "", fmt.Errorf("failed to get token: %s", err) + } + c.born = time.Now() + } + return c.token, nil +} + +// tokenRoundTripper implements the RoundTripper interface: adding the bearer token +// into the request header. +type tokenRoundTripper struct { + http.RoundTripper + + tokenGetter TokenGetter +} + +// RoundTrip adds the bearer token into the request. +func (t *tokenRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) { + // if the authorization header already present, use it. + if req.Header.Get("Authorization") != "" { + return t.RoundTripper.RoundTrip(req) + } + + token, err := t.tokenGetter.Token() + if err == nil { + req.Header.Set("Authorization", "Bearer "+token) + } else { + glog.V(4).Infof("failed to get token: %s", err) + } + + return t.RoundTripper.RoundTrip(req) +} + +// newOpenstackAuthProvider creates an auth provider which works with openstack +// environment. +func newOpenstackAuthProvider(clusterAddress string, config map[string]string, persister restclient.AuthProviderConfigPersister) (restclient.AuthProvider, error) { + var ttlDuration time.Duration + var err error + + ttl, found := config["ttl"] + if !found { + ttlDuration = DefaultTTLDuration + // persist to config + config["ttl"] = ttlDuration.String() + if err = persister.Persist(config); err != nil { + return nil, fmt.Errorf("failed to persist config: %s", err) + } + } else { + ttlDuration, err = time.ParseDuration(ttl) + if err != nil { + return nil, fmt.Errorf("failed to parse ttl config: %s", err) + } + } + + // TODO: read/persist client configuration(OS_XXX env vars) in config + + return &openstackAuthProvider{ + ttl: ttlDuration, + tokenGetter: &tokenGetter{}, + }, nil +} + +func (oap *openstackAuthProvider) WrapTransport(rt http.RoundTripper) http.RoundTripper { + return &tokenRoundTripper{ + RoundTripper: rt, + tokenGetter: &cachedGetter{ + tokenGetter: oap.tokenGetter, + ttl: oap.ttl, + }, + } +} + +func (oap *openstackAuthProvider) Login() error { return nil } diff --git a/staging/src/k8s.io/client-go/plugin/pkg/client/auth/openstack/openstack_test.go b/staging/src/k8s.io/client-go/plugin/pkg/client/auth/openstack/openstack_test.go new file mode 100644 index 0000000000000..411bec70f7f08 --- /dev/null +++ b/staging/src/k8s.io/client-go/plugin/pkg/client/auth/openstack/openstack_test.go @@ -0,0 +1,116 @@ +/* +Copyright 2017 The Kubernetes Authors. + +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 openstack + +import ( + "math/rand" + "net/http" + "testing" + "time" +) + +// testTokenGetter is a simple random token getter. +type testTokenGetter struct{} + +const LetterBytes = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ" + +func RandStringBytes(n int) string { + b := make([]byte, n) + for i := range b { + b[i] = LetterBytes[rand.Intn(len(LetterBytes))] + } + return string(b) +} + +func (*testTokenGetter) Token() (string, error) { + return RandStringBytes(32), nil +} + +// testRoundTripper is mocked roundtripper which responds with unauthorized when +// there is no authorization header, otherwise returns status ok. +type testRoundTripper struct{} + +func (trt *testRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) { + authHeader := req.Header.Get("Authorization") + if authHeader == "" || authHeader == "Bearer " { + return &http.Response{ + StatusCode: http.StatusUnauthorized, + }, nil + } + return &http.Response{StatusCode: http.StatusOK}, nil +} + +func TestOpenstackAuthProvider(t *testing.T) { + trt := &tokenRoundTripper{ + RoundTripper: &testRoundTripper{}, + } + + tests := []struct { + name string + ttl time.Duration + interval time.Duration + same bool + }{ + { + name: "normal", + ttl: 2 * time.Second, + interval: 1 * time.Second, + same: true, + }, + { + name: "expire", + ttl: 1 * time.Second, + interval: 2 * time.Second, + same: false, + }, + } + + for _, test := range tests { + trt.tokenGetter = &cachedGetter{ + tokenGetter: &testTokenGetter{}, + ttl: test.ttl, + } + + req, err := http.NewRequest(http.MethodPost, "https://test-api-server.com", nil) + if err != nil { + t.Errorf("failed to new request: %s", err) + } + trt.RoundTrip(req) + header := req.Header.Get("Authorization") + if header == "" { + t.Errorf("expect to see token in header, but is absent") + } + + time.Sleep(test.interval) + + req, err = http.NewRequest(http.MethodPost, "https://test-api-server.com", nil) + if err != nil { + t.Errorf("failed to new request: %s", err) + } + trt.RoundTrip(req) + newHeader := req.Header.Get("Authorization") + if newHeader == "" { + t.Errorf("expect to see token in header, but is absent") + } + + same := newHeader == header + if same != test.same { + t.Errorf("expect to get %t when compare header, but saw %t", test.same, same) + } + } + +} From 68cecc1ecb4fd8d7d438ff7d020dc1260647841f Mon Sep 17 00:00:00 2001 From: FengyunPan Date: Sun, 6 Aug 2017 14:29:47 +0800 Subject: [PATCH 4/5] [OpenStack] Add more detail error message I get same simple error messages "Unable to initialize cinder client for region: RegionOne" from controller-manager, but I can not find the reason. We should add more detail message "err" into glog.Errorf. --- pkg/cloudprovider/providers/openstack/openstack_client.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/cloudprovider/providers/openstack/openstack_client.go b/pkg/cloudprovider/providers/openstack/openstack_client.go index 71b0078bfd06e..916c4a09c563f 100644 --- a/pkg/cloudprovider/providers/openstack/openstack_client.go +++ b/pkg/cloudprovider/providers/openstack/openstack_client.go @@ -28,7 +28,7 @@ func (os *OpenStack) NewNetworkV2() (*gophercloud.ServiceClient, error) { Region: os.region, }) if err != nil { - glog.Warningf("Failed to find network v2 endpoint: %v", err) + glog.Warningf("Failed to find network v2 endpoint for region %s: %v", os.region, err) return nil, err } return network, nil @@ -39,7 +39,7 @@ func (os *OpenStack) NewComputeV2() (*gophercloud.ServiceClient, error) { Region: os.region, }) if err != nil { - glog.Warningf("Failed to find compute v2 endpoint: %v", err) + glog.Warningf("Failed to find compute v2 endpoint for region %s: %v", os.region, err) return nil, err } return compute, nil @@ -50,7 +50,7 @@ func (os *OpenStack) NewBlockStorageV1() (*gophercloud.ServiceClient, error) { Region: os.region, }) if err != nil { - glog.Errorf("Unable to initialize cinder v1 client for region: %s", os.region) + glog.Errorf("Unable to initialize cinder v1 client for region %s: %v", os.region, err) return nil, err } return storage, nil @@ -61,7 +61,7 @@ func (os *OpenStack) NewBlockStorageV2() (*gophercloud.ServiceClient, error) { Region: os.region, }) if err != nil { - glog.Errorf("Unable to initialize cinder v2 client for region: %s", os.region) + glog.Errorf("Unable to initialize cinder v2 client for region %s: %v", os.region, err) return nil, err } return storage, nil From 5d09d31e67a8e4c251b83e7ea884fb286a240bd3 Mon Sep 17 00:00:00 2001 From: FengyunPan Date: Mon, 7 Aug 2017 17:11:40 +0800 Subject: [PATCH 5/5] Ignore the available volume when calling DetachDisk If use detachs the volume by nova in openstack env, volume becomes available. If nova instance is been deleted, nova will detach it automatically. So the "available" is fine since that means the volume is detached from instance already. --- .../providers/openstack/openstack_volumes.go | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/pkg/cloudprovider/providers/openstack/openstack_volumes.go b/pkg/cloudprovider/providers/openstack/openstack_volumes.go index 7f7b499bb7744..a5479a4f4b61f 100644 --- a/pkg/cloudprovider/providers/openstack/openstack_volumes.go +++ b/pkg/cloudprovider/providers/openstack/openstack_volumes.go @@ -215,11 +215,7 @@ func (os *OpenStack) AttachDisk(instanceID, volumeID string) (string, error) { if err != nil { return "", err } - if volume.Status != VolumeAvailableStatus { - errmsg := fmt.Sprintf("volume %s status is %s, not %s, can not be attached to instance %s.", volume.Name, volume.Status, VolumeAvailableStatus, instanceID) - glog.Errorf(errmsg) - return "", errors.New(errmsg) - } + cClient, err := os.NewComputeV2() if err != nil { return "", err @@ -258,6 +254,12 @@ func (os *OpenStack) DetachDisk(instanceID, volumeID string) error { if err != nil { return err } + if volume.Status == VolumeAvailableStatus { + // "available" is fine since that means the volume is detached from instance already. + glog.V(2).Infof("volume: %s has been detached from compute: %s ", volume.ID, instanceID) + return nil + } + if volume.Status != VolumeInUseStatus { errmsg := fmt.Sprintf("can not detach volume %s, its status is %s.", volume.Name, volume.Status) glog.Errorf(errmsg)