From a4a8e9f551a7a185792ba5f053d0dfa96db213ad Mon Sep 17 00:00:00 2001 From: George Hicken Date: Wed, 13 May 2020 03:06:29 +0000 Subject: [PATCH 1/2] Handle lint errors in generated serivce code This adds the generated VIC service bindings to the ingore set for linting due to package names containing underscores. This will present on all but the first build in a given build env. --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index ef531a2c51..165e113d56 100644 --- a/Makefile +++ b/Makefile @@ -212,7 +212,7 @@ whitespace: @infra/scripts/whitespace-check.sh # exit 1 if golint complains about anything other than comments -golintf = $(GOLINT) $(1) | sh -c "! grep -v 'lib/apiservers/portlayer/restapi/operations'" | sh -c "! grep -v 'lib/config/dynamic/admiral/client'" | sh -c "! grep -v 'should have comment'" | sh -c "! grep -v 'comment on exported'" | sh -c "! grep -v 'by other packages, and that stutters'" | sh -c "! grep -v 'error strings should not be capitalized'" +golintf = $(GOLINT) $(1) | sh -c "! grep -v 'lib/apiservers/service/restapi/operations'" | sh -c "! grep -v 'lib/apiservers/portlayer/restapi/operations'" | sh -c "! grep -v 'lib/config/dynamic/admiral/client'" | sh -c "! grep -v 'should have comment'" | sh -c "! grep -v 'comment on exported'" | sh -c "! grep -v 'by other packages, and that stutters'" | sh -c "! grep -v 'error strings should not be capitalized'" golint: $(GOLINT) @echo checking go lint... From 5f70fbd3095cf38b367ca6f3d90ebac1d8350ec4 Mon Sep 17 00:00:00 2001 From: George Hicken Date: Wed, 13 May 2020 03:59:00 +0000 Subject: [PATCH 2/2] Name field is guest visible The Name field was changed to be hidden from the guest in #4134 to support docker rename. This was a compromise needed due to an ESX vigor bug, see #5533. This *requires* that users be on a version of ESX that has the fix or bad things may happen to cVM configurations. I have not tracked down what build versions that entails and have not code a check mechanism. This change has the effect of the hostname of the VCH endpointVM being set to the name of the VCH. It also allows additional live reconfiguration such as hotadd to a bridge network, in general modification of all container configuration live is possible, if not necessarily applied. --- lib/config/executor/container_vm.go | 2 +- lib/migration/feature/feature.go | 9 ++ lib/migration/plugins/init.go | 1 + .../plugina/guest_visible_name_appliance.go | 88 +++++++++++++++++++ .../plugina/guest_visible_name_container.go | 83 +++++++++++++++++ lib/migration/plugins/plugina/types.go | 59 +++++++++++++ lib/portlayer/exec/handle.go | 4 +- 7 files changed, 244 insertions(+), 2 deletions(-) create mode 100644 lib/migration/plugins/plugina/guest_visible_name_appliance.go create mode 100644 lib/migration/plugins/plugina/guest_visible_name_container.go create mode 100644 lib/migration/plugins/plugina/types.go diff --git a/lib/config/executor/container_vm.go b/lib/config/executor/container_vm.go index b616e08e66..e91fb597eb 100644 --- a/lib/config/executor/container_vm.go +++ b/lib/config/executor/container_vm.go @@ -78,7 +78,7 @@ type ExecutorConfigCommon struct { ID string `vic:"0.1" scope:"read-only" key:"id"` // Convenience field to record a human readable name - Name string `vic:"0.1" scope:"hidden" key:"name"` + Name string `vic:"0.1" scope:"read-only" key:"name"` // Freeform notes related to the entity Notes string `vic:"0.1" scope:"hidden" key:"notes"` diff --git a/lib/migration/feature/feature.go b/lib/migration/feature/feature.go index 485f688720..3451303e90 100644 --- a/lib/migration/feature/feature.go +++ b/lib/migration/feature/feature.go @@ -32,6 +32,15 @@ const ( // VM folder support for the VCH. VCHFolderSupportVersion + // Requires minimum ESX patch versions to support live update of guest visible + // fields while VM is running. + // Migrates Name back to guest visible - undoes this specific portion of the + // AddCommonSpecForContainerVersion migration. + // There are two plugin versions because we cannot combine changes to appliance + // and container configs + ContainerGuestVisibleName + ApplianceGuestVisibleName + // Add new feature flag here // MaxPluginVersion must be the last diff --git a/lib/migration/plugins/init.go b/lib/migration/plugins/init.go index 93bc64759b..1d98988926 100644 --- a/lib/migration/plugins/init.go +++ b/lib/migration/plugins/init.go @@ -23,4 +23,5 @@ import ( _ "github.com/vmware/vic/lib/migration/plugins/plugin7" _ "github.com/vmware/vic/lib/migration/plugins/plugin8" _ "github.com/vmware/vic/lib/migration/plugins/plugin9" + _ "github.com/vmware/vic/lib/migration/plugins/plugina" ) diff --git a/lib/migration/plugins/plugina/guest_visible_name_appliance.go b/lib/migration/plugins/plugina/guest_visible_name_appliance.go new file mode 100644 index 0000000000..b9e372cbf3 --- /dev/null +++ b/lib/migration/plugins/plugina/guest_visible_name_appliance.go @@ -0,0 +1,88 @@ +// Copyright 2017 VMware, Inc. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package plugina + +import ( + "context" + "fmt" + + log "github.com/Sirupsen/logrus" + + "github.com/vmware/vic/lib/migration/errors" + "github.com/vmware/vic/lib/migration/feature" + "github.com/vmware/vic/lib/migration/manager" + "github.com/vmware/vic/pkg/trace" + "github.com/vmware/vic/pkg/vsphere/extraconfig" + "github.com/vmware/vic/pkg/vsphere/session" +) + +const ( + atarget = manager.ApplianceConfigure +) + +func init() { + defer trace.End(trace.Begin(fmt.Sprintf("Registering plugins %s:%d", atarget, feature.ApplianceGuestVisibleName))) + + if err := manager.Migrator.Register(feature.ApplianceGuestVisibleName, atarget, &ApplianceGuestVisibleName{}); err != nil { + log.Errorf("Failed to register plugin %s:%d, %s", atarget, feature.ApplianceGuestVisibleName, err) + panic(err) + } +} + +// VCHGuestVisibleName is plugin for vic 1.5.6 version upgrade +type ApplianceGuestVisibleName struct { +} + +// Migrate is almost identical to the Migrate for containers, but with the UpdatedExecutorConfig nested into +// the VCH config. This could likely be avoid using extraconfig.DecodeWithPrefix but I chose to follow the +// pattern from prior plugins. +func (p *ApplianceGuestVisibleName) Migrate(ctx context.Context, s *session.Session, data interface{}) error { + defer trace.End(trace.Begin(fmt.Sprintf("ApplianceGuestVisibleName version: %d", feature.ApplianceGuestVisibleName))) + if data == nil { + return nil + } + mapData, ok := data.(map[string]string) + if !ok { + // Log the error here and return nil so that other plugins can proceed + log.Errorf("Migration data format is not map: %+v", data) + return nil + } + oldStruct := &VirtualContainerHostConfigSpec{} + result := extraconfig.Decode(extraconfig.MapSource(mapData), oldStruct) + log.Debugf("The oldStruct is %+v", oldStruct) + if result == nil { + return &errors.DecodeError{Err: fmt.Errorf("decode oldStruct %+v failed", oldStruct)} + } + + newStruct := &UpdatedVCHConfigSpec{ + UpdatedExecutorConfig: UpdatedExecutorConfig{ + UpdatedCommon: UpdatedCommon{ + Name: oldStruct.Name, + ID: oldStruct.ID, + ExecutionEnvironment: oldStruct.ExecutionEnvironment, + Notes: oldStruct.Notes, + }, + }, + } + + cfg := make(map[string]string) + extraconfig.Encode(extraconfig.MapSink(cfg), newStruct) + + for k, v := range cfg { + log.Debugf("New data: %s:%s", k, v) + mapData[k] = v + } + return nil +} diff --git a/lib/migration/plugins/plugina/guest_visible_name_container.go b/lib/migration/plugins/plugina/guest_visible_name_container.go new file mode 100644 index 0000000000..0205c5f79e --- /dev/null +++ b/lib/migration/plugins/plugina/guest_visible_name_container.go @@ -0,0 +1,83 @@ +// Copyright 2017 VMware, Inc. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package plugina + +import ( + "context" + "fmt" + + log "github.com/Sirupsen/logrus" + + "github.com/vmware/vic/lib/migration/errors" + "github.com/vmware/vic/lib/migration/feature" + "github.com/vmware/vic/lib/migration/manager" + "github.com/vmware/vic/pkg/trace" + "github.com/vmware/vic/pkg/vsphere/extraconfig" + "github.com/vmware/vic/pkg/vsphere/session" +) + +const ( + ctarget = manager.ContainerConfigure +) + +func init() { + defer trace.End(trace.Begin(fmt.Sprintf("Registering plugins %s:%d", ctarget, feature.ContainerGuestVisibleName))) + + if err := manager.Migrator.Register(feature.ContainerGuestVisibleName, ctarget, &ContainerGuestVisibleName{}); err != nil { + log.Errorf("Failed to register plugin %s:%d, %s", ctarget, feature.ContainerGuestVisibleName, err) + panic(err) + } +} + +// ContainerGuestVisibleName is plugin for vic 1.5.6 version upgrade +type ContainerGuestVisibleName struct { +} + +// Migrate deals with the container config and is distinct from the Migrate which is for VCH config. +func (p *ContainerGuestVisibleName) Migrate(ctx context.Context, s *session.Session, data interface{}) error { + defer trace.End(trace.Begin(fmt.Sprintf("ContainerGuestVisibleName version %d", feature.ContainerGuestVisibleName))) + if data == nil { + return nil + } + mapData, ok := data.(map[string]string) + if !ok { + // Log the error here and return nil so that other plugins can proceed + log.Errorf("Migration data format is not map: %+v", data) + return nil + } + oldStruct := &ExecutorConfig{} + result := extraconfig.Decode(extraconfig.MapSource(mapData), oldStruct) + log.Debugf("The oldStruct is %+v", oldStruct) + if result == nil { + return &errors.DecodeError{Err: fmt.Errorf("decode oldStruct %+v failed", oldStruct)} + } + + newStruct := &UpdatedExecutorConfig{ + UpdatedCommon: UpdatedCommon{ + Name: oldStruct.Name, + ID: oldStruct.ID, + Notes: oldStruct.Notes, + ExecutionEnvironment: oldStruct.ExecutionEnvironment, + }} + + cfg := make(map[string]string) + extraconfig.Encode(extraconfig.MapSink(cfg), newStruct) + + for k, v := range cfg { + log.Debugf("New data: %s:%s", k, v) + mapData[k] = v + } + return nil +} diff --git a/lib/migration/plugins/plugina/types.go b/lib/migration/plugins/plugina/types.go new file mode 100644 index 0000000000..4b2f5f54af --- /dev/null +++ b/lib/migration/plugins/plugina/types.go @@ -0,0 +1,59 @@ +// Copyright 2017 VMware, Inc. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package plugina + +type ExecutorConfig struct { + Common `vic:"0.1" scope:"read-only" key:"common"` +} + +type Common struct { + // A reference to the components hosting execution environment, if any + ExecutionEnvironment string + + // Unambiguous ID with meaning in the context of its hosting execution environment + ID string `vic:"0.1" scope:"read-only" key:"id"` + + // Convenience field to record a human readable name + Name string `vic:"0.1" scope:"hidden" key:"name"` + + // Freeform notes related to the entity + Notes string `vic:"0.1" scope:"hidden" key:"notes"` +} + +type UpdatedCommon struct { + // A reference to the components hosting execution environment, if any + ExecutionEnvironment string + + // Unambiguous ID with meaning in the context of its hosting execution environment + ID string `vic:"0.1" scope:"read-only" key:"id"` + + // Convenience field to record a human readable name + Name string `vic:"0.1" scope:"read-only" key:"name"` + + // Freeform notes related to the entity + Notes string `vic:"0.1" scope:"hidden" key:"notes"` +} + +type VirtualContainerHostConfigSpec struct { + ExecutorConfig `vic:"0.1" scope:"read-only" key:"init"` +} + +type UpdatedVCHConfigSpec struct { + UpdatedExecutorConfig `vic:"0.1" scope:"read-only" key:"init"` +} + +type UpdatedExecutorConfig struct { + UpdatedCommon `vic:"0.1" scope:"read-only" key:"common"` +} diff --git a/lib/portlayer/exec/handle.go b/lib/portlayer/exec/handle.go index df470541a1..1208bca47d 100644 --- a/lib/portlayer/exec/handle.go +++ b/lib/portlayer/exec/handle.go @@ -226,7 +226,9 @@ func (h *Handle) Commit(op trace.Operation, sess *session.Session, waitTime *int // any values set with VM powered off are inherently persistent filter = ^extraconfig.NonPersistent } else { - filter = extraconfig.NonPersistent | extraconfig.Hidden + // API side changes should no longer convert persistent keys to non-persistent while VM is running + // allow write of everything + filter = ^0 } extraconfig.Encode(extraconfig.ScopeFilterSink(uint(filter), extraconfig.MapSink(cfg)), h.ExecConfig)