Skip to content

Commit

Permalink
Merge pull request kubernetes-sigs#356 from shapeblue/add-project-sup…
Browse files Browse the repository at this point in the history
…port

Add project support
  • Loading branch information
k8s-ci-robot authored May 28, 2024
2 parents a77adcb + 28faaac commit 514e4c3
Show file tree
Hide file tree
Showing 29 changed files with 589 additions and 127 deletions.
3 changes: 2 additions & 1 deletion api/v1beta1/conversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package v1beta1
import (
"context"
"fmt"

corev1 "k8s.io/api/core/v1"
conv "k8s.io/apimachinery/pkg/conversion"
"sigs.k8s.io/cluster-api-provider-cloudstack/api/v1beta3"
Expand Down Expand Up @@ -156,7 +157,7 @@ func fetchZoneIDUsingK8s(namespace string, zoneName string) (string, error) {
}

func fetchZoneIDUsingCloudStack(secret *corev1.Secret, zoneName string) (string, error) {
client, err := cloud.NewClientFromK8sSecret(secret, nil)
client, err := cloud.NewClientFromK8sSecret(secret, nil, "")
if err != nil {
return "", err
}
Expand Down
4 changes: 4 additions & 0 deletions api/v1beta3/cloudstackfailuredomain_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,10 @@ type CloudStackFailureDomainSpec struct {
// +optional
Domain string `json:"domain,omitempty"`

// CloudStack project.
// +optional
Project string `json:"project,omitempty"`

// Apache CloudStack Endpoint secret reference.
ACSEndpoint corev1.SecretReference `json:"acsEndpoint"`
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -376,6 +376,9 @@ spec:
name:
description: The failure domain unique name.
type: string
project:
description: CloudStack project.
type: string
zone:
description: The ACS Zone for this failure domain.
properties:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,9 @@ spec:
name:
description: The failure domain unique name.
type: string
project:
description: CloudStack project.
type: string
zone:
description: The ACS Zone for this failure domain.
properties:
Expand Down
1 change: 1 addition & 0 deletions controllers/cloudstackaffinitygroup_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ var _ = Describe("CloudStackAffinityGroupReconciler", func() {
mockCloudClient.EXPECT().FetchAffinityGroup(gomock.Any()).Do(func(arg1 interface{}) {
arg1.(*cloud.AffinityGroup).ID = ""
}).AnyTimes().Return(nil)
Ω(k8sClient.Delete(ctx, dummies.CSAffinityGroup))

Ω(k8sClient.Delete(ctx, dummies.CSAffinityGroup))

Expand Down
4 changes: 2 additions & 2 deletions controllers/utils/failuredomains.go
Original file line number Diff line number Diff line change
Expand Up @@ -147,12 +147,12 @@ func (c *CloudClientImplementation) AsFailureDomainUser(fdSpec *infrav1.CloudSta
_ = c.K8sClient.Get(c.RequestCtx, key, clientConfig)

var err error
if c.CSClient, err = cloud.NewClientFromK8sSecret(endpointCredentials, clientConfig); err != nil {
if c.CSClient, err = cloud.NewClientFromK8sSecret(endpointCredentials, clientConfig, fdSpec.Project); err != nil {
return ctrl.Result{}, errors.Wrapf(err, "parsing ACSEndpoint secret with ref: %v", fdSpec.ACSEndpoint)
}

if fdSpec.Account != "" { // Set r.CSUser CloudStack Client per Account and Domain.
client, err := c.CSClient.NewClientInDomainAndAccount(fdSpec.Domain, fdSpec.Account)
client, err := c.CSClient.NewClientInDomainAndAccount(fdSpec.Domain, fdSpec.Account, fdSpec.Project)
if err != nil {
return ctrl.Result{}, err
}
Expand Down
9 changes: 6 additions & 3 deletions pkg/cloud/affinity_groups.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package cloud

import (
"github.com/apache/cloudstack-go/v2/cloudstack"
"github.com/pkg/errors"
infrav1 "sigs.k8s.io/cluster-api-provider-cloudstack/api/v1beta3"
)
Expand All @@ -42,7 +43,7 @@ type AffinityGroupIface interface {

func (c *client) FetchAffinityGroup(group *AffinityGroup) (reterr error) {
if group.ID != "" {
affinityGroup, count, err := c.cs.AffinityGroup.GetAffinityGroupByID(group.ID)
affinityGroup, count, err := c.cs.AffinityGroup.GetAffinityGroupByID(group.ID, cloudstack.WithProject(c.user.Project.ID))
if err != nil {
// handle via multierr
c.customMetrics.EvaluateErrorAndIncrementAcsReconciliationErrorCounter(err)
Expand All @@ -57,7 +58,7 @@ func (c *client) FetchAffinityGroup(group *AffinityGroup) (reterr error) {
}
}
if group.Name != "" {
affinityGroup, count, err := c.cs.AffinityGroup.GetAffinityGroupByName(group.Name)
affinityGroup, count, err := c.cs.AffinityGroup.GetAffinityGroupByName(group.Name, cloudstack.WithProject(c.user.Project.ID))
if err != nil {
// handle via multierr
c.customMetrics.EvaluateErrorAndIncrementAcsReconciliationErrorCounter(err)
Expand All @@ -78,6 +79,7 @@ func (c *client) GetOrCreateAffinityGroup(group *AffinityGroup) (retErr error) {
if err := c.FetchAffinityGroup(group); err != nil { // Group not found?
p := c.cs.AffinityGroup.NewCreateAffinityGroupParams(group.Name, group.Type)
p.SetName(group.Name)
setIfNotEmpty(c.user.Project.ID, p.SetProjectid)
resp, err := c.cs.AffinityGroup.CreateAffinityGroup(p)
if err != nil {
c.customMetrics.EvaluateErrorAndIncrementAcsReconciliationErrorCounter(err)
Expand All @@ -92,6 +94,7 @@ func (c *client) DeleteAffinityGroup(group *AffinityGroup) (retErr error) {
p := c.cs.AffinityGroup.NewDeleteAffinityGroupParams()
setIfNotEmpty(group.ID, p.SetId)
setIfNotEmpty(group.Name, p.SetName)
setIfNotEmpty(c.user.Project.ID, p.SetProjectid)
_, retErr = c.cs.AffinityGroup.DeleteAffinityGroup(p)
c.customMetrics.EvaluateErrorAndIncrementAcsReconciliationErrorCounter(retErr)
return retErr
Expand All @@ -101,7 +104,7 @@ type affinityGroups []AffinityGroup

func (c *client) getCurrentAffinityGroups(csMachine *infrav1.CloudStackMachine) (affinityGroups, error) {
// Start by fetching VM details which includes an array of currently associated affinity groups.
if virtM, count, err := c.cs.VirtualMachine.GetVirtualMachineByID(*csMachine.Spec.InstanceID); err != nil {
if virtM, count, err := c.cs.VirtualMachine.GetVirtualMachineByID(*csMachine.Spec.InstanceID, cloudstack.WithProject(c.user.Project.ID)); err != nil {
c.customMetrics.EvaluateErrorAndIncrementAcsReconciliationErrorCounter(err)
return nil, err
} else if count > 1 {
Expand Down
18 changes: 9 additions & 9 deletions pkg/cloud/affinity_groups_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,21 +58,21 @@ var _ = Describe("AffinityGroup Unit Tests", func() {
Context("Fetch or Create Affinity group", func() {
It("fetches an affinity group by Name", func() {
dummies.AffinityGroup.ID = "" // Force name fetching.
ags.EXPECT().GetAffinityGroupByName(dummies.AffinityGroup.Name).Return(&cloudstack.AffinityGroup{}, 1, nil)
ags.EXPECT().GetAffinityGroupByName(dummies.AffinityGroup.Name, gomock.Any()).Return(&cloudstack.AffinityGroup{}, 1, nil)

Ω(client.GetOrCreateAffinityGroup(dummies.AffinityGroup)).Should(Succeed())
})

It("fetches an affinity group by ID", func() {
ags.EXPECT().GetAffinityGroupByID(dummies.AffinityGroup.ID).Return(&cloudstack.AffinityGroup{}, 1, nil)
ags.EXPECT().GetAffinityGroupByID(dummies.AffinityGroup.ID, gomock.Any()).Return(&cloudstack.AffinityGroup{}, 1, nil)

Ω(client.GetOrCreateAffinityGroup(dummies.AffinityGroup)).Should(Succeed())
})

It("creates an affinity group", func() {
// dummies.SetDummyDomainAndAccount()
// dummies.SetDummyDomainID()
ags.EXPECT().GetAffinityGroupByID(dummies.AffinityGroup.ID).Return(nil, -1, fakeError)
ags.EXPECT().GetAffinityGroupByID(dummies.AffinityGroup.ID, gomock.Any()).Return(nil, -1, fakeError)
ags.EXPECT().NewCreateAffinityGroupParams(dummies.AffinityGroup.Name, dummies.AffinityGroup.Type).
Return(&cloudstack.CreateAffinityGroupParams{})
ags.EXPECT().CreateAffinityGroup(ParamMatch(And(NameEquals(dummies.AffinityGroup.Name)))).
Expand All @@ -84,7 +84,7 @@ var _ = Describe("AffinityGroup Unit Tests", func() {
It("creates an affinity group if Name provided returns more than one affinity group", func() {
dummies.AffinityGroup.ID = "" // Force name fetching.
agp := &cloudstack.CreateAffinityGroupParams{}
ags.EXPECT().GetAffinityGroupByName(dummies.AffinityGroup.Name).Return(&cloudstack.AffinityGroup{}, 2, nil)
ags.EXPECT().GetAffinityGroupByName(dummies.AffinityGroup.Name, gomock.Any()).Return(&cloudstack.AffinityGroup{}, 2, nil)
ags.EXPECT().NewCreateAffinityGroupParams(gomock.Any(), gomock.Any()).Return(agp)
ags.EXPECT().CreateAffinityGroup(agp).Return(&cloudstack.CreateAffinityGroupResponse{}, nil)

Expand All @@ -94,7 +94,7 @@ var _ = Describe("AffinityGroup Unit Tests", func() {
It("creates an affinity group if getting affinity group by name fails", func() {
dummies.AffinityGroup.ID = "" // Force name fetching.
agp := &cloudstack.CreateAffinityGroupParams{}
ags.EXPECT().GetAffinityGroupByName(dummies.AffinityGroup.Name).Return(nil, -1, fakeError)
ags.EXPECT().GetAffinityGroupByName(dummies.AffinityGroup.Name, gomock.Any()).Return(nil, -1, fakeError)
ags.EXPECT().NewCreateAffinityGroupParams(gomock.Any(), gomock.Any()).Return(agp)
ags.EXPECT().CreateAffinityGroup(agp).Return(&cloudstack.CreateAffinityGroupResponse{}, nil)

Expand All @@ -103,7 +103,7 @@ var _ = Describe("AffinityGroup Unit Tests", func() {

It("creates an affinity group if ID provided returns more than one affinity group", func() {
agp := &cloudstack.CreateAffinityGroupParams{}
ags.EXPECT().GetAffinityGroupByID(dummies.AffinityGroup.ID).Return(&cloudstack.AffinityGroup{}, 2, nil)
ags.EXPECT().GetAffinityGroupByID(dummies.AffinityGroup.ID, gomock.Any()).Return(&cloudstack.AffinityGroup{}, 2, nil)
ags.EXPECT().NewCreateAffinityGroupParams(gomock.Any(), gomock.Any()).Return(agp)
ags.EXPECT().CreateAffinityGroup(agp).Return(&cloudstack.CreateAffinityGroupResponse{}, nil)

Expand All @@ -112,7 +112,7 @@ var _ = Describe("AffinityGroup Unit Tests", func() {

It("creates an affinity group if getting affinity group by ID fails", func() {
agp := &cloudstack.CreateAffinityGroupParams{}
ags.EXPECT().GetAffinityGroupByID(dummies.AffinityGroup.ID).Return(nil, -1, fakeError)
ags.EXPECT().GetAffinityGroupByID(dummies.AffinityGroup.ID, gomock.Any()).Return(nil, -1, fakeError)
ags.EXPECT().NewCreateAffinityGroupParams(gomock.Any(), gomock.Any()).Return(agp)
ags.EXPECT().CreateAffinityGroup(agp).Return(&cloudstack.CreateAffinityGroupResponse{}, nil)

Expand Down Expand Up @@ -164,7 +164,7 @@ var _ = Describe("AffinityGroup Unit Tests", func() {
It("Associate affinity group", func() {
uagp := &cloudstack.UpdateVMAffinityGroupParams{}
vmp := &cloudstack.StartVirtualMachineParams{}
vms.EXPECT().GetVirtualMachineByID(*dummies.CSMachine1.Spec.InstanceID).Return(&cloudstack.VirtualMachine{}, 1, nil)
vms.EXPECT().GetVirtualMachineByID(*dummies.CSMachine1.Spec.InstanceID, gomock.Any()).Return(&cloudstack.VirtualMachine{}, 1, nil)
ags.EXPECT().NewUpdateVMAffinityGroupParams(*dummies.CSMachine1.Spec.InstanceID).Return(uagp)
vms.EXPECT().NewStopVirtualMachineParams(*dummies.CSMachine1.Spec.InstanceID).Return(&cloudstack.StopVirtualMachineParams{})
vms.EXPECT().StopVirtualMachine(&cloudstack.StopVirtualMachineParams{}).Return(&cloudstack.StopVirtualMachineResponse{State: "Stopping"}, nil)
Expand All @@ -177,7 +177,7 @@ var _ = Describe("AffinityGroup Unit Tests", func() {
It("Disassociate affinity group", func() {
uagp := &cloudstack.UpdateVMAffinityGroupParams{}
vmp := &cloudstack.StartVirtualMachineParams{}
vms.EXPECT().GetVirtualMachineByID(*dummies.CSMachine1.Spec.InstanceID).Return(&cloudstack.VirtualMachine{}, 1, nil)
vms.EXPECT().GetVirtualMachineByID(*dummies.CSMachine1.Spec.InstanceID, gomock.Any()).Return(&cloudstack.VirtualMachine{}, 1, nil)
ags.EXPECT().NewUpdateVMAffinityGroupParams(*dummies.CSMachine1.Spec.InstanceID).Return(uagp)
vms.EXPECT().NewStopVirtualMachineParams(*dummies.CSMachine1.Spec.InstanceID).Return(&cloudstack.StopVirtualMachineParams{})
vms.EXPECT().StopVirtualMachine(&cloudstack.StopVirtualMachineParams{}).Return(&cloudstack.StopVirtualMachineResponse{State: "Stopping"}, nil)
Expand Down
22 changes: 13 additions & 9 deletions pkg/cloud/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ type Client interface {
ZoneIFace
IsoNetworkIface
UserCredIFace
NewClientInDomainAndAccount(string, string) (Client, error)
NewClientInDomainAndAccount(string, string, string) (Client, error)
}

// cloud-config ini structure.
Expand Down Expand Up @@ -101,7 +101,7 @@ func UnmarshalAllSecretConfigs(in []byte, out *[]SecretConfig) error {
}

// NewClientFromK8sSecret returns a client from a k8s secret
func NewClientFromK8sSecret(endpointSecret *corev1.Secret, clientConfig *corev1.ConfigMap) (Client, error) {
func NewClientFromK8sSecret(endpointSecret *corev1.Secret, clientConfig *corev1.ConfigMap, project string) (Client, error) {
endpointSecretStrings := map[string]string{}
for k, v := range endpointSecret.Data {
endpointSecretStrings[k] = string(v)
Expand All @@ -110,19 +110,19 @@ func NewClientFromK8sSecret(endpointSecret *corev1.Secret, clientConfig *corev1.
if err != nil {
return nil, err
}
return NewClientFromBytesConfig(bytes, clientConfig)
return NewClientFromBytesConfig(bytes, clientConfig, project)
}

// NewClientFromBytesConfig returns a client from a bytes array that unmarshals to a yaml config.
func NewClientFromBytesConfig(conf []byte, clientConfig *corev1.ConfigMap) (Client, error) {
func NewClientFromBytesConfig(conf []byte, clientConfig *corev1.ConfigMap, project string) (Client, error) {
r := bytes.NewReader(conf)
dec := yaml.NewDecoder(r)
var config Config
if err := dec.Decode(&config); err != nil {
return nil, err
}

return NewClientFromConf(config, clientConfig)
return NewClientFromConf(config, clientConfig, project)
}

// NewClientFromYamlPath returns a client from a yaml config at path.
Expand All @@ -146,11 +146,11 @@ func NewClientFromYamlPath(confPath string, secretName string) (Client, error) {
return nil, errors.Errorf("config with secret name %s not found", secretName)
}

return NewClientFromConf(conf, nil)
return NewClientFromConf(conf, nil, "")
}

// NewClientFromConf creates a new Cloud Client form a map of strings to strings.
func NewClientFromConf(conf Config, clientConfig *corev1.ConfigMap) (Client, error) {
func NewClientFromConf(conf Config, clientConfig *corev1.ConfigMap, project string) (Client, error) {
cacheMutex.Lock()
defer cacheMutex.Unlock()

Expand Down Expand Up @@ -189,6 +189,9 @@ func NewClientFromConf(conf Config, clientConfig *corev1.ConfigMap) (Client, err
ID: userResponse.Users[0].Domainid,
},
},
Project: Project{
Name: project,
},
}
if found, err := c.GetUserWithKeys(user); err != nil {
return nil, err
Expand All @@ -203,10 +206,11 @@ func NewClientFromConf(conf Config, clientConfig *corev1.ConfigMap) (Client, err
}

// NewClientInDomainAndAccount returns a new client in the specified domain and account.
func (c *client) NewClientInDomainAndAccount(domain string, account string) (Client, error) {
func (c *client) NewClientInDomainAndAccount(domain string, account string, project string) (Client, error) {
user := &User{}
user.Account.Domain.Path = domain
user.Account.Name = account
user.Project.Name = project
if found, err := c.GetUserWithKeys(user); err != nil {
return nil, err
} else if !found {
Expand All @@ -217,7 +221,7 @@ func (c *client) NewClientInDomainAndAccount(domain string, account string) (Cli
c.config.SecretKey = user.SecretKey
c.user = user

return NewClientFromConf(c.config, nil)
return NewClientFromConf(c.config, nil, project)
}

// NewClientFromCSAPIClient creates a client from a CloudStack-Go API client. Used only for testing.
Expand Down
10 changes: 5 additions & 5 deletions pkg/cloud/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ var _ = Describe("Client", func() {
config := cloud.Config{
APIUrl: "http://1.1.1.1",
}
result, err := cloud.NewClientFromConf(config, clientConfig)
result, err := cloud.NewClientFromConf(config, clientConfig, "")
Ω(err).ShouldNot(HaveOccurred())
Ω(result).ShouldNot(BeNil())
})
Expand All @@ -174,8 +174,8 @@ var _ = Describe("Client", func() {
config2 := cloud.Config{
APIUrl: "http://3.3.3.3",
}
result1, _ := cloud.NewClientFromConf(config1, clientConfig)
result2, _ := cloud.NewClientFromConf(config2, clientConfig)
result1, _ := cloud.NewClientFromConf(config1, clientConfig, "")
result2, _ := cloud.NewClientFromConf(config2, clientConfig, "")
Ω(result1).ShouldNot(Equal(result2))
})

Expand All @@ -186,8 +186,8 @@ var _ = Describe("Client", func() {
config2 := cloud.Config{
APIUrl: "http://4.4.4.4",
}
result1, _ := cloud.NewClientFromConf(config1, clientConfig)
result2, _ := cloud.NewClientFromConf(config2, clientConfig)
result1, _ := cloud.NewClientFromConf(config1, clientConfig, "")
result2, _ := cloud.NewClientFromConf(config2, clientConfig, "")
Ω(result1).Should(Equal(result2))
})
})
Expand Down
2 changes: 1 addition & 1 deletion pkg/cloud/cloud_suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ func TestCloud(t *testing.T) {

// Switch to test account user.
realCloudClient, connectionErr = realCloudClient.NewClientInDomainAndAccount(
newAccount.Domain.Name, newAccount.Name)
newAccount.Domain.Name, newAccount.Name, "")
Ω(connectionErr).ShouldNot(HaveOccurred())
}
})
Expand Down
Loading

0 comments on commit 514e4c3

Please sign in to comment.