Skip to content
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

Jump to bottom

Bug 1779933: set version to start for s390/ppc until we have actual samples #205

Merged
merged 1 commit into from Dec 5, 2019
Merged

Bug 1779933: set version to start for s390/ppc until we have actual samples #205

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion pkg/operatorstatus/operatorstatus.go
View file
Open in desktop
Expand Up @@ -87,7 +87,7 @@ func (o *ClusterOperatorHandler) UpdateOperatorStatus(cfg *v1.Config, deletionIn
// will ignore errors in delete path, but we at least log them above
return nil
}
if len(cfg.Spec.Architectures) > 0 && cfg.Spec.Architectures[0] != v1.AMDArchitecture && cfg.Spec.Architectures[0] != v1.X86Architecture {
if util.IsNonX86Arch(cfg) {
o.setOperatorStatusWithoutInterrogatingConfig(configv1.ConditionFalse, cfg, nonX86)
// will ignore errors in non-x86 path, but we at least log them above
return nil
Expand Down
6 changes: 5 additions & 1 deletion pkg/stub/handler.go
View file
Open in desktop
Expand Up @@ -588,6 +588,10 @@ func (h *Handler) Handle(event util.Event) error {
// Every time we see a change to the Config object, update the ClusterOperator status
// based on the current conditions of the Config.
cfg = h.refetchCfgMinimizeConflicts(cfg)
//TODO remove this setting of version once we start getting samples for z or ppc
if util.IsNonX86Arch(cfg) {
cfg.Status.Version = h.version
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't we always set this, and vary whether or not it is reported based on the Available status?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no we shouldn't ... all that was sorted out in 4.0/4.1 initial dev

only report the version once you have achieved it

}
err := h.cvowrapper.UpdateOperatorStatus(cfg, false)
if err != nil {
logrus.Errorf("error updating cluster operator status: %v", err)
Expand Down Expand Up @@ -625,7 +629,7 @@ func (h *Handler) Handle(event util.Event) error {
}

//TODO adjust this check as we start getting samples for z or ppc
if len(cfg.Spec.Architectures) > 0 && cfg.Spec.Architectures[0] != v1.AMDArchitecture && cfg.Spec.Architectures[0] != v1.X86Architecture {
if util.IsNonX86Arch(cfg) {
logrus.Printf("samples are not installed on non-x86 architectures")
return nil
}
Expand Down
31 changes: 27 additions & 4 deletions pkg/stub/handler_test.go
View file
Open in desktop
Expand Up @@ -3,6 +3,7 @@ package stub
import (
"fmt"
"os"
"runtime"
"strings"
"time"

Expand Down Expand Up @@ -83,9 +84,17 @@ func TestWithDist(t *testing.T) {
func TestWithArchDist(t *testing.T) {
h, cfg, event := setup()
processCred(&h, cfg, t)
cfg.Spec.Architectures = []string{
v1.X86Architecture,
if len(cfg.Spec.Architectures) == 0 {
t.Errorf("arch not set on bootstrap")
}
testValue := runtime.GOARCH
if testValue == v1.AMDArchitecture {
testValue = v1.X86Architecture
}
if cfg.Spec.Architectures[0] != testValue {
t.Errorf("arch set to %s instead of %s", cfg.Spec.Architectures[0], runtime.GOARCH)
}

Copy link
Contributor

Choose a reason for hiding this comment

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

It doesn't look like validate verifies that we've set version correctly. Needs to be updated.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the setting of the version happens after the setup() call ... it does not need to be in validate

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we don't need it everwhere

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nevermind ... I'll do it everywhere

Copy link
Contributor Author

Choose a reason for hiding this comment

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

unit test updated

mimic(&h, x86OCPContentRootDir)
err := h.Handle(event)
statuses := []corev1.ConditionStatus{corev1.ConditionFalse, corev1.ConditionTrue, corev1.ConditionTrue, corev1.ConditionTrue, corev1.ConditionFalse, corev1.ConditionFalse, corev1.ConditionFalse}
Expand All @@ -98,6 +107,7 @@ func TestWithArchDist(t *testing.T) {
conditions,
statuses, t)


Copy link
Member

Choose a reason for hiding this comment

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

nit: why do we keep adding empty lines?

}

func TestWithArch(t *testing.T) {
Expand All @@ -108,7 +118,7 @@ func TestWithArch(t *testing.T) {
v1.PPCArchitecture,
}
err := h.Handle(event)
validate(true, err, "", cfg, conditions, []corev1.ConditionStatus{corev1.ConditionFalse, corev1.ConditionTrue, corev1.ConditionTrue, corev1.ConditionFalse, corev1.ConditionFalse, corev1.ConditionFalse, corev1.ConditionFalse}, t)
validateArchOverride(true, err, "", cfg, conditions, []corev1.ConditionStatus{corev1.ConditionFalse, corev1.ConditionTrue, corev1.ConditionTrue, corev1.ConditionFalse, corev1.ConditionFalse, corev1.ConditionFalse, corev1.ConditionFalse}, t, v1.PPCArchitecture)
}

func TestWithBadArch(t *testing.T) {
Expand Down Expand Up @@ -1280,7 +1290,7 @@ func mimic(h *Handler, topdir string) {

}

func validate(succeed bool, err error, errstr string, cfg *v1.Config, statuses []v1.ConfigConditionType, conditions []corev1.ConditionStatus, t *testing.T) {
func validateArchOverride(succeed bool, err error, errstr string, cfg *v1.Config, statuses []v1.ConfigConditionType, conditions []corev1.ConditionStatus, t *testing.T, arch string) {
if succeed && err != nil {
t.Fatal(err)
}
Expand All @@ -1307,9 +1317,22 @@ func validate(succeed bool, err error, errstr string, cfg *v1.Config, statuses [
t.Fatalf("unexpected for succeed %v have status condition %#v expected condition %#v and status %#v", succeed, cfg.Status.Conditions[i], c, conditions[i])
}
}
testValue := arch
if testValue == v1.AMDArchitecture {
testValue = v1.X86Architecture
}
if cfg.Spec.Architectures[0] != testValue {
t.Fatalf("arch set to %s instead of %s", cfg.Spec.Architectures[0], runtime.GOARCH)
}
}

}

func validate(succeed bool, err error, errstr string, cfg *v1.Config, statuses []v1.ConfigConditionType, conditions []corev1.ConditionStatus, t *testing.T) {
validateArchOverride(succeed, err, errstr, cfg, statuses, conditions, t, runtime.GOARCH)
}


func NewTestHandler() Handler {
h := Handler{}

Expand Down
8 changes: 8 additions & 0 deletions pkg/util/util.go
View file
Open in desktop
Expand Up @@ -325,6 +325,14 @@ func ClusterNeedsCreds(s *samplev1.Config, ) bool {
return ConditionFalse(s, samplev1.ImportCredentialsExist)
}

// IsNonX86Arch let's us know if this is something other than x86_64/amd like s390x or ppc
func IsNonX86Arch(cfg *samplev1.Config) bool {
if len(cfg.Spec.Architectures) > 0 && cfg.Spec.Architectures[0] != samplev1.AMDArchitecture && cfg.Spec.Architectures[0] != samplev1.X86Architecture {
return true
}
return false
}

type Event struct {
Object runtime.Object
Deleted bool
Expand Down