Change precedence of bundle annotations to override command-line flag values.

Previously, flag values from bundle annotations would have the second-lowest
precedence:

- Most important:  Pod flag annotation
- More important:  Command-line flag
- Less important:  Bundle flag
- Least important: Flag default

Now, bundle flags move up one level in precedence:

- Most important:  Pod flag annotation
- More important:  Bundle flag
- Less important:  Command-line flag
- Least important: Flag default

This precedence change is useful in the context of containerd installations,
where command-line flag values are actually "default-like", in the sense that
they are always specified whenever `runsc` is invoked, and the binary flag
defaults don't really matter. In such a setting (which is the setting bundle
annotations were targeting), something having lower precedence than
command-line flags may as well not exist.

With this change, this puts bundle annotations closer to their flag annotation
counterpart, while still allowing user-controlled flag annotations to
ultimately decide the final configuration.

PiperOrigin-RevId: 515179596

runsc/config/config_test.go

@@ -599,7 +599,7 @@ func TestBundles(t *testing.T) {
 			},
 		},
 		{
-			Name: "command line takes precedence to non-default value",
+			Name: "bundle takes precedence over command-line value",
 			BundleConfig: map[BundleName]Bundle{
 				"no-debug": {
 					"debug": "false",
@@ -609,26 +609,25 @@ func TestBundles(t *testing.T) {
 			Bundles:     []BundleName{"no-debug"},
 			Verify: func(t *testing.T, old, new *Config) {
 				t.Helper()
-				noChange(t, old, new)
-				if !new.Debug {
-					t.Error("debug was not set to true")
+				if new.Debug {
+					t.Error("debug is still true")
 				}
 			},
 		},
 		{
-			Name: "command line takes precedence to default value",
+			Name: "command line matching bundle value",
 			BundleConfig: map[BundleName]Bundle{
 				"debug": {
 					"debug": "true",
 				},
 			},
-			CommandLine: []string{"-debug=false"},
+			CommandLine: []string{"-debug=true"},
 			Bundles:     []BundleName{"debug"},
 			Verify: func(t *testing.T, old, new *Config) {
 				t.Helper()
 				noChange(t, old, new)
-				if new.Debug {
-					t.Error("debug was set to true")
+				if !new.Debug {
+					t.Error("debug was set to false")
 				}
 			},
 		},
@@ -656,7 +655,10 @@ func TestBundles(t *testing.T) {
 			if !test.WantErr && err != nil {
 				t.Errorf("got unexpected error: %v", err)
 			}
-			if err != nil && test.Verify != nil && !t.Failed() {
+			if t.Failed() {
+				return
+			}
+			if err != nil && test.Verify != nil {
 				t.Error("cannot specify Verify function for erroring tests")
 			}
 			if err == nil && test.Verify != nil {

runsc/config/flags.go

@@ -392,7 +392,7 @@ func (c *Config) isOverrideAllowed(name string, value string) error {
 // ApplyBundles applies the given bundles by name.
 // It returns an error if a bundle doesn't exist, or if the given
 // bundles have conflicting flag values.
-// Config values which are already specified prior to calling ApplyBundles do not change.
+// Config values which are already specified prior to calling ApplyBundles are overridden.
 func (c *Config) ApplyBundles(flagSet *flag.FlagSet, bundleNames ...BundleName) error {
 	// Populate a map from flag name to flag value to bundle name.
 	flagToValueToBundleName := make(map[string]map[string]BundleName)
@@ -443,18 +443,17 @@ func (c *Config) ApplyBundles(flagSet *flag.FlagSet, bundleNames ...BundleName)
 		// Note: We verified earlier that valueToBundleName has length 1,
 		// so this loop executes exactly once per flag.
 		for val, bundleName := range valueToBundleName {
-			if isFlagExplicitlySet(flagSet, flagName) {
-				if prevValue != val {
-					log.Infof("Bundle %s is supposed to have the effect of setting flag --%s to %q, but this flag was also explicitly set to --%s=%q on the command-line; the command-line value --%s=%q takes precedence.", bundleName, flagName, val, flagName, prevValue, flagName, prevValue)
-				}
+			if prevValue == val {
 				continue
 			}
+			if isFlagExplicitlySet(flagSet, flagName) {
+				log.Infof("Flag --%s has explicitly-set value %q, but bundle %s takes precedence and is overriding its value to --%s=%q.", flagName, prevValue, bundleName, flagName, val)
+			} else {
+				log.Infof("Overriding flag --%s=%q from applying bundle %s.", flagName, val, bundleName)
+			}
 			if err := c.Override(flagSet, flagName, val /* force= */, true); err != nil {
 				return err
 			}
-			if prevValue != val {
-				log.Infof("Applying bundle %s: flag --%s has been updated from --%s=%q to --%s=%q", bundleName, flagName, flagName, prevValue, flagName, val)
-			}
 		}
 	}
 

runsc/specutils/specutils.go

@@ -254,7 +254,7 @@ func fixSpec(spec *specs.Spec, bundleDir string, conf *config.Config) error {
 			// Override flags using annotation to allow customization per sandbox
 			// instance.
 			name := annotation[len(annotationFlagPrefix):]
-			log.Infof("Overriding flag: %s=%q", name, val)
+			log.Infof("Overriding flag from flag annotation: --%s=%q", name, val)
 			if err := conf.Override(flag.CommandLine, name, val /* force= */, false); err != nil {
 				return err
 			}