Fix server downloading inactive kontainers

daxmc99 · Craig Jellick · commit a251b0b39bef · 2019-02-15T12:16:32.000-07:00
Problem: Server would download drivers even if they were marked inactive. This throws errors on air-gapped installations. Solution: Server now only downloads files when they are marked as active. Modify test to conform to this also. rancher#17595
diff --git a/pkg/controllers/management/drivers/kontainerdriver/kontainerdriver.go b/pkg/controllers/management/drivers/kontainerdriver/kontainerdriver.go
@@ -55,13 +55,16 @@ type Lifecycle struct {
 func (l *Lifecycle) Create(obj *v3.KontainerDriver) (runtime.Object, error) {
 	logrus.Infof("create kontainerdriver %v", obj.Name)
 
-	v3.KontainerDriverConditionDownloaded.Unknown(obj)
-	v3.KontainerDriverConditionInstalled.Unknown(obj)
-
+	// return early if driver is not active
+	// set driver to a non-transitioning state
 	if !obj.Spec.Active {
+		v3.KontainerDriverConditionInactive.True(obj)
 		return obj, nil
 	}
 
+	v3.KontainerDriverConditionDownloaded.Unknown(obj)
+	v3.KontainerDriverConditionInstalled.Unknown(obj)
+
 	// Update status
 	obj, err := l.kontainerDrivers.Update(obj)
 	if err != nil {
@@ -351,15 +354,17 @@ func (l *Lifecycle) Updated(obj *v3.KontainerDriver) (runtime.Object, error) {
 		return obj, l.updateDynamicSchema(obj)
 	}
 
-	// redownload file if url changed or not downloaded
+	// redownload file if active AND url changed or not downloaded
+	// prevents downloading on air-gapped installations
 	var err error
-	if obj.Spec.URL != obj.Status.ActualURL || v3.KontainerDriverConditionDownloaded.IsFalse(obj) || !l.driverExists(obj) {
+	if obj.Spec.Active && (obj.Spec.URL != obj.Status.ActualURL || v3.KontainerDriverConditionDownloaded.IsFalse(obj) || !l.driverExists(obj)) {
 		obj, err = l.download(obj)
 		if err != nil {
 			return nil, err
 		}
 	}
-
+	// Create schema if obj is active
+	// Delete schema if obj is inactive
 	if obj.Spec.Active {
 		err = l.createDynamicSchema(obj)
 
@@ -368,7 +373,6 @@ func (l *Lifecycle) Updated(obj *v3.KontainerDriver) (runtime.Object, error) {
 		if err = l.dynamicSchemas.Delete(getDynamicTypeName(obj), &v13.DeleteOptions{}); err != nil && !errors.IsNotFound(err) {
 			return nil, fmt.Errorf("error deleting schema: %v", err)
 		}
-
 		if err = l.removeFieldFromCluster(obj); err != nil {
 			return nil, err
 		}
@@ -424,6 +428,7 @@ func (l *Lifecycle) Remove(obj *v3.KontainerDriver) (runtime.Object, error) {
 	return obj, nil
 }
 
+// Removes the v3/dynamicschema "cluster"'s resourceField associated with the KontainerDriver obj
 func (l *Lifecycle) removeFieldFromCluster(obj *v3.KontainerDriver) error {
 	nodedriver.SchemaLock.Lock()
 	defer nodedriver.SchemaLock.Unlock()
@@ -432,7 +437,7 @@ func (l *Lifecycle) removeFieldFromCluster(obj *v3.KontainerDriver) error {
 
 	nodeSchema, err := l.dynamicSchemasLister.Get("", "cluster")
 	if err != nil {
-		return fmt.Errorf("error getting schema: %v", err)
+		return fmt.Errorf("error getting schema: %v", err) // this error may fire during Rancher startup
 	}
 
 	nodeSchema = nodeSchema.DeepCopy()
diff --git a/tests/core/test_kontainer_drivers.py b/tests/core/test_kontainer_drivers.py
@@ -64,14 +64,16 @@ def test_kontainer_driver_lifecycle(admin_mc, remove_resource):
 
 @pytest.mark.nonparallel
 def test_enabling_driver_exposes_schema(admin_mc, remove_resource):
+    """ Test if enabling driver exposes its dynamic schema, drivers are
+     downloaded / installed once they are active """
     kd = admin_mc.client.create_kontainerDriver(
         createDynamicSchema=True,
         active=False,
         url=DRIVER_URL
     )
     remove_resource(kd)
 
-    kd = wait_for_condition('Installed', 'True', admin_mc.client, kd,
+    kd = wait_for_condition('Inactive', 'True', admin_mc.client, kd,
                             timeout=90)
 
     # verify the kontainer driver has no activate and a deactivate link
@@ -81,7 +83,7 @@ def test_enabling_driver_exposes_schema(admin_mc, remove_resource):
 
     verify_driver_not_in_types(admin_mc.client, kd)
 
-    kd.active = True
+    kd.active = True  # driver should begin downloading / installing
     admin_mc.client.update_by_id_kontainerDriver(kd.id, kd)
 
     kd = wait_for_condition('Active', 'True', admin_mc.client, kd,
