Expose more test framework functions #1205
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -40,36 +40,45 @@ import ( | |
| ) | ||
|
|
||
| var ( | ||
| // Global is a global framework struct that the test can use. | ||
| Global *Framework | ||
| // mutex for AddToFrameworkScheme | ||
| mutex = sync.Mutex{} | ||
| // singleNamespaceInternal determines whether tests are to be run in a single namespace or not. | ||
| singleNamespaceInternal bool | ||
| // decoder used by createFromYaml | ||
| dynamicDecoder runtime.Decoder | ||
| // restMapper for the dynamic client | ||
| restMapper *restmapper.DeferredDiscoveryRESTMapper | ||
| ) | ||
|
|
||
| // Framework contains all relevant variables needed for running tests with the operator-sdk. | ||
| type Framework struct { | ||
| Client *frameworkClient | ||
| KubeConfig *rest.Config | ||
| KubeClient kubernetes.Interface | ||
| Scheme *runtime.Scheme | ||
| NamespacedManPath string | ||
|
There was a problem hiding this comment. Was this done initially in case we could have a nil and do we not have that case anymore? There was a problem hiding this comment. I'm not sure why it was a pointer. We don't have any nil checks anywhere, so this doesn't affect anything. Just makes more sense to have it as a normal string IMO There was a problem hiding this comment. Makes sense, will this be a breaking change for the users, should we explicitly mention it in the changelog. There was a problem hiding this comment. I don't think it's something the users should have been accessing directly in the first place. It might make sense to unexport this field (and maybe a few others) There was a problem hiding this comment. Hi @AlexNPavel, I think that namespace can be null, is not possible the user/dev use it as an lib/imported in their project? |
||
| Namespace string | ||
| LocalOperator bool | ||
| } | ||
|
|
||
| // Setup initializes the Global.Framework variable and its fields. | ||
| func Setup(kubeconfigPath, namespacedManPath, namespace string, singleNamespace, localOperator bool) error { | ||
|
There was a problem hiding this comment. I'm not sure if there are problematic implications, but can this function signature be refactored to something like: type FrameworkOptions struct {
KubeconfigPath string
NamespacedManPath string
Namespace string
SingleNamespace bool
LocalOperator bool
}
func NewFramework(opts FrameworkOptions) (*Framework, error)Then I realize this would be a breaking change, so that's worth some discussion. I think we should try to avoid having global mutable state as much as possible and favor returning and passing values around instead. There was a problem hiding this comment. The biggest difficulty with not having the global There was a problem hiding this comment. One way to avoid the global state in There was a problem hiding this comment. Ok I think I'm understanding this a little bit better now having taken a look at https://github.com/operator-framework/operator-sdk/blob/master/test/test-framework/test/e2e/memcached_test.go. If I understand correctly, right now it's basically required for tests to call If getting rid of the exported |
||
| if namespace != "" && !singleNamespace { | ||
| return fmt.Errorf("oneNamespace must be set to true if namespace is set") | ||
| } | ||
| singleNamespaceInternal = singleNamespace | ||
| var err error | ||
| var kubeconfig *rest.Config | ||
| if kubeconfigPath == "incluster" { | ||
| // when running with an InCluster config, we don't have permission to create new namespaces, so we must be in single namespace mode | ||
| if singleNamespaceInternal != true { | ||
| return fmt.Errorf("singleNamespace must be set to true for in cluster testing mode") | ||
| } | ||
| if len(namespace) == 0 { | ||
| return fmt.Errorf("namespace must be set for in cluster testing mode") | ||
| } | ||
| // Work around https://github.com/kubernetes/kubernetes/issues/40973 | ||
| if len(os.Getenv("KUBERNETES_SERVICE_HOST")) == 0 { | ||
| addrs, err := net.LookupHost("kubernetes.default.svc") | ||
|
|
@@ -86,15 +95,10 @@ func setup(kubeconfigPath, namespacedManPath *string, localOperator bool) error | |
| } | ||
| } | ||
| kubeconfig, err = rest.InClusterConfig() | ||
| } else { | ||
| var kcNamespace string | ||
| kubeconfig, kcNamespace, err = k8sInternal.GetKubeconfigAndNamespace(kubeconfigPath) | ||
| if singleNamespaceInternal && namespace == "" { | ||
| namespace = kcNamespace | ||
| } | ||
| } | ||
|
|
@@ -147,34 +151,29 @@ type addToSchemeFunc func(*runtime.Scheme) error | |
| // } | ||
| // The List object is needed because the CRD has not always been fully registered | ||
| // by the time this function is called. If the CRD takes more than 5 seconds to | ||
| // become ready, this function throws an error. | ||
| func AddToFrameworkScheme(addToScheme addToSchemeFunc, obj runtime.Object) error { | ||
| mutex.Lock() | ||
| defer mutex.Unlock() | ||
| err := addToScheme(Global.Scheme) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to update global scheme: %v", err) | ||
| } | ||
| restMapper.Reset() | ||
| err = wait.PollImmediate(time.Second, time.Second*10, func() (done bool, err error) { | ||
| if singleNamespaceInternal { | ||
| err = Global.Client.List(goctx.TODO(), &dynclient.ListOptions{Namespace: Global.Namespace}, obj) | ||
| } else { | ||
| err = Global.Client.List(goctx.TODO(), &dynclient.ListOptions{Namespace: "default"}, obj) | ||
| } | ||
| if err != nil { | ||
| restMapper.Reset() | ||
| return false, nil | ||
| } | ||
| return true, nil | ||
| }) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to update the client restmapper: %v", err) | ||
| } | ||
| dynamicDecoder = serializer.NewCodecFactory(Global.Scheme).UniversalDeserializer() | ||
| return nil | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.