Skip to content

Commit

Permalink
Merge pull request #187 from gabemontero/nonx86
Browse files Browse the repository at this point in the history
DEVEXP-418: fetch arch from GOARCH, allow for z/ppc
  • Loading branch information
openshift-merge-robot committed Oct 7, 2019
2 parents c994725 + b47b67b commit b723076
Show file tree
Hide file tree
Showing 7 changed files with 170 additions and 33 deletions.
14 changes: 12 additions & 2 deletions pkg/apis/samples/v1/types.go
Expand Up @@ -58,6 +58,16 @@ const (
// X86Architecture is the value used to specify the x86_64 hardware architecture
// in the Architectures array field.
X86Architecture = "x86_64"
// AMDArchitecture is the golang value for x86 64 bit hardware architecture; for the purposes
// of this operator, it is equivalent to X86Architecture, which is kept for historical/migration
// purposes
AMDArchitecture = "amd64"
// PPCArchitecture is the value used to specify the x86_64 hardware architecture
// in the Architectures array field.
PPCArchitecture = "ppc64le"
// S390Architecture is the value used to specify the s390x hardware architecture
// in the Architecture array field.
S390Architecture = "s390x"
// ConfigFinalizer is the text added to the Config.Finalizer field
// to enable finalizer processing.
ConfigFinalizer = GroupName + "/finalizer"
Expand Down Expand Up @@ -94,8 +104,8 @@ type ConfigSpec struct {
// defaults to registry.redhat.io.
SamplesRegistry string `json:"samplesRegistry,omitempty" protobuf:"bytes,2,opt,name=samplesRegistry"`

// Architectures determine which hardware architecture(s) to install, where x86_64 is the only
// supported choice currently.
// Architectures determine which hardware architecture(s) to install, where x86_64, ppc64le, and s390x are the only
// supported choices currently.
Architectures []string `json:"architectures,omitempty" protobuf:"bytes,4,opt,name=architectures"`

// SkippedImagestreams specifies names of image streams that should NOT be
Expand Down
39 changes: 27 additions & 12 deletions pkg/operatorstatus/operatorstatus.go
Expand Up @@ -22,6 +22,7 @@ import (
const (
ClusterOperatorName = "openshift-samples"
doingDelete = "DeletionInProgress"
nonX86 = "NonX86Platform"
)

// ClusterOperatorHandler allows for wrappering access to configv1.ClusterOperator
Expand Down Expand Up @@ -61,23 +62,37 @@ type ClusterOperatorWrapper interface {
Create(state *configv1.ClusterOperator) (err error)
}

// this method ensures that Available==true and Degraded==false, regardless of other conditions; it currently
// allows Progressing to be explicitly set to true or false based on the scenario the caller is addressing
func (o *ClusterOperatorHandler) setOperatorStatusWithoutInterrogatingConfig(progressing configv1.ConditionStatus, cfg *v1.Config, reason string) {
err := o.setOperatorStatus(configv1.OperatorAvailable, configv1.ConditionTrue, "", cfg.Status.Version, reason)
if err != nil {
logrus.Warningf("error occurred while attempting to set available condition: %s", err.Error())
}
err = o.setOperatorStatus(configv1.OperatorProgressing, progressing, "", cfg.Status.Version, reason)
if err != nil {
logrus.Warningf("error occurred while attempting to set progressing condition: %s", err.Error())
}
err = o.setOperatorStatus(configv1.OperatorDegraded, configv1.ConditionFalse, "", cfg.Status.Version, reason)
if err != nil {
logrus.Warningf("error occurred while attempting to set degraded condition: %s", err.Error())
}

}

func (o *ClusterOperatorHandler) UpdateOperatorStatus(cfg *v1.Config, deletionInProgress bool) error {
if deletionInProgress {
err := o.setOperatorStatus(configv1.OperatorAvailable, configv1.ConditionTrue, "", cfg.Status.Version, doingDelete)
if err != nil {
logrus.Warningf("error occurred while attempting to set available condition: %s", err.Error())
}
err = o.setOperatorStatus(configv1.OperatorProgressing, configv1.ConditionTrue, "", cfg.Status.Version, doingDelete)
if err != nil {
logrus.Warningf("error occurred while attempting to set progressing condition: %s", err.Error())
}
err = o.setOperatorStatus(configv1.OperatorDegraded, configv1.ConditionFalse, "", cfg.Status.Version, doingDelete)
if err != nil {
logrus.Warningf("error occurred while attempting to set degraded condition: %s", err.Error())
}
o.setOperatorStatusWithoutInterrogatingConfig(configv1.ConditionTrue, cfg, doingDelete)
// 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 {
o.setOperatorStatusWithoutInterrogatingConfig(configv1.ConditionFalse, cfg, nonX86)
// will ignore errors in non-x86 path, but we at least log them above
return nil

}

errs := []error{}
degraded, degradedReason, degradedDetail := cfg.ClusterOperatorStatusDegradedCondition()
err := o.setOperatorStatus(configv1.OperatorDegraded,
Expand Down
5 changes: 4 additions & 1 deletion pkg/stub/config.go
Expand Up @@ -30,8 +30,11 @@ func (h *Handler) SpecValidation(cfg *v1.Config) error {
for _, arch := range cfg.Spec.Architectures {
switch arch {
case v1.X86Architecture:
case v1.AMDArchitecture:
case v1.PPCArchitecture:
case v1.S390Architecture:
default:
err := fmt.Errorf("architecture %s unsupported; only support %s", arch, v1.X86Architecture)
err := fmt.Errorf("architecture %s unsupported; only support %s", arch, strings.Join([]string{v1.X86Architecture, v1.AMDArchitecture, v1.PPCArchitecture, v1.S390Architecture}, ","))
return h.processError(cfg, v1.ConfigurationValid, corev1.ConditionFalse, err, "%v")
}
}
Expand Down
6 changes: 6 additions & 0 deletions pkg/stub/files.go
Expand Up @@ -57,6 +57,12 @@ func (h *Handler) GetBaseDir(arch string, opcfg *v1.Config) (dir string) {
switch arch {
case v1.X86Architecture:
dir = x86OCPContentRootDir
case v1.AMDArchitecture:
dir = x86OCPContentRootDir
case v1.PPCArchitecture:
dir = ppcOCPContentRootDir
case v1.S390Architecture:
dir = zOCPContentRootDir
default:
}
return dir
Expand Down
66 changes: 60 additions & 6 deletions pkg/stub/handler.go
Expand Up @@ -4,6 +4,7 @@ import (
"fmt"
"io/ioutil"
"os"
"runtime"
"strings"
"sync"
"time"
Expand Down Expand Up @@ -45,6 +46,8 @@ import (

const (
x86OCPContentRootDir = "/opt/openshift/operator/ocp-x86_64"
ppcOCPContentRootDir = "/opt/openshift/operator/ocp-ppc64le"
zOCPContentRootDir = "/opt/openshift/operator/ocp-s390x"
installtypekey = "keyForInstallTypeField"
regkey = "keyForSamplesRegistryField"
skippedstreamskey = "keyForSkippedImageStreamsField"
Expand Down Expand Up @@ -277,6 +280,25 @@ func IsRetryableAPIError(err error) bool {
return false
}

// this method assumes it is only called on initial cfg create, or if the Architecture array len == 0
func (h *Handler) updateCfgArch(cfg *v1.Config) *v1.Config {
switch {
// if you look at https://golang.org/dl/ the arch symbols for ppc and 390 container
// the values of our constants below.
case strings.Contains(runtime.GOARCH, v1.PPCArchitecture):
cfg.Spec.Architectures = append(cfg.Spec.Architectures, v1.PPCArchitecture)
case strings.Contains(runtime.GOARCH, v1.S390Architecture):
cfg.Spec.Architectures = append(cfg.Spec.Architectures, v1.S390Architecture)
case strings.Contains(runtime.GOARCH, v1.AMDArchitecture):
fallthrough
case strings.Contains(runtime.GOARCH, v1.X86Architecture):
cfg.Spec.Architectures = append(cfg.Spec.Architectures, v1.X86Architecture)
default:
logrus.Warningf("unsupported hardware architecture indicated by the golang GOARCH variable being set to %s", runtime.GOARCH)
}
return cfg
}

func (h *Handler) CreateDefaultResourceIfNeeded(cfg *v1.Config) (*v1.Config, error) {
// assume the caller has call lock on the mutex .. out pattern is to have that as
// high up the stack as possible ... loc because need to
Expand Down Expand Up @@ -326,7 +348,7 @@ func (h *Handler) CreateDefaultResourceIfNeeded(cfg *v1.Config) (*v1.Config, err
cfg.Name = v1.ConfigName
cfg.Kind = "Config"
cfg.APIVersion = v1.GroupName + "/" + v1.Version
cfg.Spec.Architectures = append(cfg.Spec.Architectures, v1.X86Architecture)
cfg = h.updateCfgArch(cfg)
cfg.Spec.ManagementState = operatorsv1api.Managed
h.AddFinalizer(cfg)
// we should get a watch event for the default pull secret, but just in case
Expand Down Expand Up @@ -597,6 +619,12 @@ func (h *Handler) Handle(event v1.Event) error {
return h.crdwrapper.UpdateStatus(cfg, dbg)
}

//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 {
logrus.Printf("samples are not installed on non-x86 architectures")
return nil
}

h.buildSkipFilters(cfg)
configChanged := false
configChangeRequiresUpsert := false
Expand Down Expand Up @@ -720,7 +748,7 @@ func (h *Handler) Handle(event v1.Event) error {
}

if len(cfg.Spec.Architectures) == 0 {
cfg.Spec.Architectures = append(cfg.Spec.Architectures, v1.X86Architecture)
cfg = h.updateCfgArch(cfg)
}

h.StoreCurrentValidConfig(cfg)
Expand Down Expand Up @@ -787,10 +815,8 @@ func (h *Handler) Handle(event v1.Event) error {
now := kapis.Now()
cfg = h.refetchCfgMinimizeConflicts(cfg)
progressing := cfg.Condition(v1.ImageChangesInProgress)
progressing.LastUpdateTime = now
progressing.LastTransitionTime = now
logrus.Debugf("Handle changing processing from false to true")
progressing.Status = corev1.ConditionTrue
progressing.Reason = ""
progressing.Message = ""
for isName := range h.imagestreamFile {
_, skipped := h.skippedImagestreams[isName]
unskipping := len(unskippedStreams) > 0
Expand All @@ -802,6 +828,34 @@ func (h *Handler) Handle(event v1.Event) error {
progressing.Reason = progressing.Reason + isName + " "
}
}
if len(progressing.Reason) > 0 {
progressing.LastUpdateTime = now
progressing.LastTransitionTime = now
logrus.Debugf("Handle changing processing from false to true")
progressing.Status = corev1.ConditionTrue
} else {
logrus.Debugln("there are no imagestreams available for import")
// see if there are unskipped templates, to set SamplesExists to true
if !cfg.ConditionTrue(v1.SamplesExist) {
templatesProcessed := false
for tName := range h.templateFile {
_, skipped := h.skippedTemplates[tName]
if skipped {
continue
}
templatesProcessed = true
break
}
if templatesProcessed {
cfg = h.refetchCfgMinimizeConflicts(cfg)
h.GoodConditionUpdate(cfg, corev1.ConditionTrue, v1.SamplesExist)
dbg := "exist true update templates only"
logrus.Printf("CRDUPDATE %s", dbg)
return h.crdwrapper.UpdateStatus(cfg, dbg)
}
}
return nil
}
logrus.Debugf("Handle Reason field set to %s", progressing.Reason)
cfg.ConditionUpdate(progressing)

Expand Down
35 changes: 23 additions & 12 deletions pkg/stub/handler_test.go
Expand Up @@ -52,23 +52,29 @@ func TestNoArchOrDist(t *testing.T) {
h, cfg, event := setup()
processCred(&h, cfg, t)
err := h.Handle(event)
statuses := []corev1.ConditionStatus{corev1.ConditionFalse, corev1.ConditionTrue, corev1.ConditionTrue, corev1.ConditionTrue, corev1.ConditionFalse, corev1.ConditionFalse, corev1.ConditionFalse}
// image in progress (4th entry, array index 3) should still be false when there is no content ... a la z or ppc
statuses := []corev1.ConditionStatus{corev1.ConditionFalse, corev1.ConditionTrue, corev1.ConditionTrue, corev1.ConditionFalse, corev1.ConditionFalse, corev1.ConditionFalse, corev1.ConditionFalse}
validate(true, err, "", cfg, conditions, statuses, t)
err = h.Handle(event)
statuses[0] = corev1.ConditionTrue
// and on a subsequent event, exists should still be false since we did not create any content previously
validate(true, err, "", cfg, conditions, statuses, t)
}

func TestWithDist(t *testing.T) {
h, cfg, event := setup()
processCred(&h, cfg, t)
err := h.Handle(event)
statuses := []corev1.ConditionStatus{corev1.ConditionFalse, corev1.ConditionTrue, corev1.ConditionTrue, corev1.ConditionTrue, corev1.ConditionFalse, corev1.ConditionFalse, corev1.ConditionFalse}
statuses := []corev1.ConditionStatus{corev1.ConditionFalse, corev1.ConditionTrue, corev1.ConditionTrue, corev1.ConditionFalse, corev1.ConditionFalse, corev1.ConditionFalse, corev1.ConditionFalse}
validate(true, err, "", cfg, conditions, statuses, t)
err = h.Handle(event)
statuses = []corev1.ConditionStatus{corev1.ConditionTrue, corev1.ConditionTrue, corev1.ConditionTrue, corev1.ConditionTrue, corev1.ConditionFalse, corev1.ConditionFalse, corev1.ConditionFalse}
// image in progress (4th entry, array index 3) should still be false when there is no content ... a la z or ppc
statuses = []corev1.ConditionStatus{corev1.ConditionFalse, corev1.ConditionTrue, corev1.ConditionTrue, corev1.ConditionFalse, corev1.ConditionFalse, corev1.ConditionFalse, corev1.ConditionFalse}
validate(true, err, "", cfg, conditions, statuses, t)
mimic(&h, x86OCPContentRootDir)
err = h.Handle(event)
// with content present, image im progress should now be true
statuses = []corev1.ConditionStatus{corev1.ConditionFalse, corev1.ConditionTrue, corev1.ConditionTrue, corev1.ConditionTrue, corev1.ConditionFalse, corev1.ConditionFalse, corev1.ConditionFalse}
validate(true, err, "", cfg, conditions, statuses, t)

}

func TestWithArchDist(t *testing.T) {
Expand All @@ -94,11 +100,12 @@ func TestWithArchDist(t *testing.T) {
func TestWithArch(t *testing.T) {
h, cfg, event := setup()
processCred(&h, cfg, t)
// without a mimic call this simulates our current PPC/390 stories of no samples content
cfg.Spec.Architectures = []string{
v1.X86Architecture,
v1.PPCArchitecture,
}
err := h.Handle(event)
validate(true, err, "", cfg, conditions, []corev1.ConditionStatus{corev1.ConditionFalse, corev1.ConditionTrue, corev1.ConditionTrue, corev1.ConditionTrue, corev1.ConditionFalse, corev1.ConditionFalse, corev1.ConditionFalse}, t)
validate(true, err, "", cfg, conditions, []corev1.ConditionStatus{corev1.ConditionFalse, corev1.ConditionTrue, corev1.ConditionTrue, corev1.ConditionFalse, corev1.ConditionFalse, corev1.ConditionFalse, corev1.ConditionFalse}, t)
}

func TestWithBadArch(t *testing.T) {
Expand Down Expand Up @@ -219,7 +226,7 @@ func TestSkipped(t *testing.T) {
mimic(&h, x86OCPContentRootDir)

err := h.Handle(event)
validate(true, err, "", cfg, conditions, []corev1.ConditionStatus{corev1.ConditionFalse, corev1.ConditionTrue, corev1.ConditionTrue, corev1.ConditionTrue, corev1.ConditionFalse, corev1.ConditionFalse, corev1.ConditionFalse}, t)
validate(true, err, "", cfg, conditions, []corev1.ConditionStatus{corev1.ConditionFalse, corev1.ConditionTrue, corev1.ConditionTrue, corev1.ConditionFalse, corev1.ConditionFalse, corev1.ConditionFalse, corev1.ConditionFalse}, t)

fakeisclient := h.imageclientwrapper.(*fakeImageStreamClientWrapper)
for _, key := range iskeys {
Expand Down Expand Up @@ -485,6 +492,7 @@ func TestCreateDeleteSecretBeforeCR(t *testing.T) {
ResourceVersion: "a",
},
}
mimic(&h, x86OCPContentRootDir)

err := h.Handle(event)
validate(false, err, "Received secret samples-registry-credentials but do not have the Config yet", cfg,
Expand Down Expand Up @@ -517,7 +525,7 @@ func TestCreateDeleteSecretBeforeCR(t *testing.T) {

func TestCreateDeleteSecretAfterCR(t *testing.T) {
h, cfg, event := setup()

mimic(&h, x86OCPContentRootDir)
err := h.Handle(event)
statuses := []corev1.ConditionStatus{corev1.ConditionFalse, corev1.ConditionFalse, corev1.ConditionTrue, corev1.ConditionFalse, corev1.ConditionFalse, corev1.ConditionFalse, corev1.ConditionFalse}
validate(true, err, "", cfg, conditions, statuses, t)
Expand Down Expand Up @@ -588,7 +596,7 @@ func processCred(h *Handler, cfg *v1.Config, t *testing.T) {

func TestSameSecret(t *testing.T) {
h, cfg, event := setup()

mimic(&h, x86OCPContentRootDir)
err := h.Handle(event)
statuses := []corev1.ConditionStatus{corev1.ConditionFalse, corev1.ConditionFalse, corev1.ConditionTrue, corev1.ConditionFalse, corev1.ConditionFalse, corev1.ConditionFalse, corev1.ConditionFalse}
validate(true, err, "", cfg, conditions, statuses, t)
Expand Down Expand Up @@ -981,20 +989,22 @@ func TestDeletedCR(t *testing.T) {

func TestSameCR(t *testing.T) {
h, cfg, event := setup()
mimic(&h, x86OCPContentRootDir)
processCred(&h, cfg, t)
cfg.ResourceVersion = "a"

// first pass on the resource creates the samples
// first pass on the resource creates the samples, exists (first entry, index 0) is still false
statuses := []corev1.ConditionStatus{corev1.ConditionFalse, corev1.ConditionTrue, corev1.ConditionTrue, corev1.ConditionTrue, corev1.ConditionFalse, corev1.ConditionFalse, corev1.ConditionFalse}
err := h.Handle(event)
validate(true, err, "", cfg, conditions, statuses, t)

err = h.Handle(event)
// next pass is when we expect exists to be true
statuses[0] = corev1.ConditionTrue
validate(true, err, "", cfg, conditions, statuses, t)

err = h.Handle(event)
statuses[3] = corev1.ConditionFalse
// now with content, should see no change in status after duplicate event, where imagestream import status has not changed
validate(true, err, "", cfg, conditions, statuses, t)

}
Expand Down Expand Up @@ -1022,6 +1032,7 @@ func TestBadSubDirList(t *testing.T) {

func TestBadTopLevelStatus(t *testing.T) {
h, cfg, event := setup()
mimic(&h, x86OCPContentRootDir)
processCred(&h, cfg, t)
fakestatus := h.crdwrapper.(*fakeCRDWrapper)
fakestatus.updateerr = fmt.Errorf("badsdkupdate")
Expand Down

0 comments on commit b723076

Please sign in to comment.