Skip to content
This repository has been archived by the owner on Jan 11, 2023. It is now read-only.

acs-engine orchestrators should throw an error when an invalid/unsup… #3118

Merged
merged 1 commit into from
Jun 4, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
22 changes: 22 additions & 0 deletions pkg/api/common/const.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,29 @@ const (
OpenShiftDefaultVersion string = OpenShiftVersion3Dot9Dot0
)

const (
// SwarmVersion is the Swarm orchestrator version
SwarmVersion = "swarm:1.1.0"
// DockerCEVersion is the DockerCE orchestrator version
DockerCEVersion = "17.03.*"
)

// GetAllSupportedDCOSVersions returns a slice of all supported DCOS versions.
func GetAllSupportedDCOSVersions() []string {
return AllDCOSSupportedVersions
}

// GetAllSupportedOpenShiftVersions returns a slice of all supported OpenShift versions.
func GetAllSupportedOpenShiftVersions() []string {
return []string{OpenShiftVersion3Dot9Dot0, OpenShiftVersionUnstable}
}

// GetAllSupportedSwarmVersions returns a slice of all supported Swarm versions.
func GetAllSupportedSwarmVersions() []string {
return []string{SwarmVersion}
}

// GetAllSupportedDockerCEVersions returns a slice of all supported Docker CE versions.
func GetAllSupportedDockerCEVersions() []string {
return []string{DockerCEVersion}
}
79 changes: 58 additions & 21 deletions pkg/api/orchestrators.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
type orchestratorsFunc func(*OrchestratorProfile) ([]*OrchestratorVersionProfile, error)

var funcmap map[string]orchestratorsFunc
var versionsMap map[string][]string

func init() {
funcmap = map[string]orchestratorsFunc{
Expand All @@ -22,6 +23,13 @@ func init() {
SwarmMode: dockerceInfo,
OpenShift: openShiftInfo,
}
versionsMap = map[string][]string{
Copy link
Contributor

Choose a reason for hiding this comment

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

even better :)

Kubernetes: common.GetAllSupportedKubernetesVersions(),
DCOS: common.GetAllSupportedDCOSVersions(),
Swarm: common.GetAllSupportedSwarmVersions(),
SwarmMode: common.GetAllSupportedDockerCEVersions(),
OpenShift: common.GetAllSupportedOpenShiftVersions(),
}
}

func validate(orchestrator, version string) (string, error) {
Expand All @@ -46,6 +54,18 @@ func validate(orchestrator, version string) (string, error) {
return "", nil
}

func isVersionSupported(csOrch *OrchestratorProfile) bool {
supported := false
for _, version := range versionsMap[csOrch.OrchestratorType] {

if version == csOrch.OrchestratorVersion {
supported = true
break
}
}
return supported
}

// GetOrchestratorVersionProfileListVLabs returns vlabs OrchestratorVersionProfileList object per (optionally) specified orchestrator and version
func GetOrchestratorVersionProfileListVLabs(orchestrator, version string) (*vlabs.OrchestratorVersionProfileList, error) {
apiOrchs, err := getOrchestratorVersionProfileList(orchestrator, version)
Expand Down Expand Up @@ -89,7 +109,7 @@ func getOrchestratorVersionProfileList(orchestrator, version string) ([]*Orchest
orchs = append(orchs, arr...)
}
} else {
if orchs, err = funcmap[orchestrator](&OrchestratorProfile{OrchestratorVersion: version}); err != nil {
if orchs, err = funcmap[orchestrator](&OrchestratorProfile{OrchestratorType: orchestrator, OrchestratorVersion: version}); err != nil {
return nil, err
}
}
Expand Down Expand Up @@ -137,15 +157,7 @@ func kubernetesInfo(csOrch *OrchestratorProfile) ([]*OrchestratorVersionProfile,
})
}
} else {
ver, err := semver.NewVersion(csOrch.OrchestratorVersion)
if err != nil {
return nil, err
}
cons, err := semver.NewConstraint("<1.6.0")
if err != nil {
return nil, err
}
if cons.Check(ver) {
if !isVersionSupported(csOrch) {
return nil, fmt.Errorf("Kubernetes version %s is not supported", csOrch.OrchestratorVersion)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this code redundant now?

Copy link
Contributor

Choose a reason for hiding this comment

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

@tariq1890 I think you can remove the check for "<1.6.0" since those will return false for !isVersionSupported(csOrch)

}

Expand Down Expand Up @@ -204,6 +216,10 @@ func dcosInfo(csOrch *OrchestratorProfile) ([]*OrchestratorVersionProfile, error
})
}
} else {
if !isVersionSupported(csOrch) {
return nil, fmt.Errorf("DCOS version %s is not supported", csOrch.OrchestratorVersion)
}

// get info for the specified version
upgrades, err := dcosUpgrades(csOrch)
if err != nil {
Expand Down Expand Up @@ -236,22 +252,51 @@ func dcosUpgrades(csOrch *OrchestratorProfile) ([]*OrchestratorProfile, error) {
}

func swarmInfo(csOrch *OrchestratorProfile) ([]*OrchestratorVersionProfile, error) {
if csOrch.OrchestratorVersion == "" {
return []*OrchestratorVersionProfile{
{
OrchestratorProfile: OrchestratorProfile{
OrchestratorType: Swarm,
OrchestratorVersion: SwarmVersion,
},
},
}, nil
}

if !isVersionSupported(csOrch) {
return nil, fmt.Errorf("Swarm version %s is not supported", csOrch.OrchestratorVersion)
}
return []*OrchestratorVersionProfile{
{
OrchestratorProfile: OrchestratorProfile{
OrchestratorType: Swarm,
OrchestratorVersion: SwarmVersion,
OrchestratorVersion: csOrch.OrchestratorVersion,
},
},
}, nil
}

func dockerceInfo(csOrch *OrchestratorProfile) ([]*OrchestratorVersionProfile, error) {

if csOrch.OrchestratorVersion == "" {
return []*OrchestratorVersionProfile{
{
OrchestratorProfile: OrchestratorProfile{
OrchestratorType: SwarmMode,
OrchestratorVersion: DockerCEVersion,
},
},
}, nil
}

if !isVersionSupported(csOrch) {
return nil, fmt.Errorf("Docker CE version %s is not supported", csOrch.OrchestratorVersion)
}
return []*OrchestratorVersionProfile{
{
OrchestratorProfile: OrchestratorProfile{
OrchestratorType: SwarmMode,
OrchestratorVersion: DockerCEVersion,
OrchestratorVersion: csOrch.OrchestratorVersion,
},
},
}, nil
Expand All @@ -276,15 +321,7 @@ func openShiftInfo(csOrch *OrchestratorProfile) ([]*OrchestratorVersionProfile,
})
}
} else {
ver, err := semver.NewVersion(csOrch.OrchestratorVersion)
if err != nil {
return nil, err
}
cons, err := semver.NewConstraint("<3.9.0")
if err != nil {
return nil, err
}
if cons.Check(ver) {
if !isVersionSupported(csOrch) {
return nil, fmt.Errorf("OpenShift version %s is not supported", csOrch.OrchestratorVersion)
}

Expand Down
97 changes: 93 additions & 4 deletions pkg/api/orchestrators_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,8 @@ func TestVersionCompare(t *testing.T) {

func TestOrchestratorUpgradeInfo(t *testing.T) {
RegisterTestingT(t)
// 1.6.8 is upgradable to 1.6.x and 1.7.x
deployedVersion := "1.6.8"
// 1.6.9 is upgradable to 1.6.x and 1.7.x
deployedVersion := "1.6.9"
nextNextMinorVersion := "1.8.0"
csOrch := &OrchestratorProfile{
OrchestratorType: Kubernetes,
Expand Down Expand Up @@ -164,7 +164,9 @@ func TestKubernetesInfo(t *testing.T) {
"31.29.",
".17.02",
"43.156.89.",
"1.2.a"}
"1.2.a",
"1.5.9",
"1.6.8"}

for _, v := range invalid {
csOrch := &OrchestratorProfile{
Expand All @@ -188,7 +190,9 @@ func TestOpenshiftInfo(t *testing.T) {
"31.29.",
".17.02",
"43.156.89.",
"1.2.a"}
"1.2.a",
"3.8.9",
"3.9.2"}

for _, v := range invalid {
csOrch := &OrchestratorProfile{
Expand All @@ -209,3 +213,88 @@ func TestOpenshiftInfo(t *testing.T) {
_, e := openShiftInfo(csOrch)
Expect(e).To(BeNil())
}

func TestDcosInfo(t *testing.T) {
RegisterTestingT(t)
invalid := []string{
"invalid number",
"invalid.number",
"a4.b7.c3",
"31.29.",
".17.02",
"43.156.89.",
"1.2.a"}

for _, v := range invalid {
csOrch := &OrchestratorProfile{
OrchestratorType: DCOS,
OrchestratorVersion: v,
}

_, e := dcosInfo(csOrch)
Expect(e).NotTo(BeNil())
}

// test good value
csOrch := &OrchestratorProfile{
OrchestratorType: DCOS,
OrchestratorVersion: common.DCOSDefaultVersion,
}

_, e := dcosInfo(csOrch)
Expect(e).To(BeNil())
}

func TestSwarmInfo(t *testing.T) {
RegisterTestingT(t)
invalid := []string{
"swarm:1.1.1",
"swarm:1.1.2",
}

for _, v := range invalid {
csOrch := &OrchestratorProfile{
OrchestratorType: Swarm,
OrchestratorVersion: v,
}

_, e := swarmInfo(csOrch)
Expect(e).NotTo(BeNil())
}

// test good value
csOrch := &OrchestratorProfile{
OrchestratorType: Swarm,
OrchestratorVersion: common.SwarmVersion,
}

_, e := swarmInfo(csOrch)
Expect(e).To(BeNil())
}

func TestDockerceInfoInfo(t *testing.T) {
RegisterTestingT(t)
invalid := []string{
"17.02.1",
"43.156.89",
}

for _, v := range invalid {
csOrch := &OrchestratorProfile{
OrchestratorType: SwarmMode,
OrchestratorVersion: v,
}

_, e := dockerceInfo(csOrch)
Expect(e).NotTo(BeNil())
}

// test good value
csOrch := &OrchestratorProfile{
OrchestratorType: SwarmMode,
OrchestratorVersion: common.DockerCEVersion,
}

_, e := dockerceInfo(csOrch)
Expect(e).To(BeNil())
}
4 changes: 2 additions & 2 deletions pkg/armhelpers/mockclients.go
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ func (mc *MockACSEngineClient) ListVirtualMachines(resourceGroup string) (comput
poolnameString := "poolName"

creationSource := "acsengine-k8s-agentpool1-12345678-0"
orchestrator := "Kubernetes:1.6.8"
orchestrator := "Kubernetes:1.6.9"
resourceNameSuffix := "12345678"
poolname := "agentpool1"

Expand Down Expand Up @@ -272,7 +272,7 @@ func (mc *MockACSEngineClient) GetVirtualMachine(resourceGroup, name string) (co
poolnameString := "poolName"

creationSource := "acsengine-k8s-agentpool1-12345678-0"
orchestrator := "Kubernetes:1.6.8"
orchestrator := "Kubernetes:1.6.9"
resourceNameSuffix := "12345678"
poolname := "agentpool1"

Expand Down
2 changes: 1 addition & 1 deletion pkg/operations/kubernetesupgrade/upgradecluster_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ var _ = Describe("Upgrade Kubernetes cluster tests", func() {

err := uc.UpgradeCluster(subID, "kubeConfig", "TestRg", cs, "12345678", []string{"agentpool1"}, TestACSEngineVersion)
Expect(err).NotTo(BeNil())
Expect(err.Error()).To(Equal("Error while querying ARM for resources: Kubernetes:1.6.8 cannot be upgraded to 1.8.6"))
Expect(err.Error()).To(Equal("Error while querying ARM for resources: Kubernetes:1.6.9 cannot be upgraded to 1.8.6"))
})

It("Should return error message when failing to delete role assignment during upgrade operation", func() {
Expand Down