Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

operator: use infra and network manifests to create controllerconfigspec #357

Merged
merged 8 commits into from
Feb 14, 2019
Merged

operator: use infra and network manifests to create controllerconfigspec #357

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
88d3e94
apis: replace ClusterName, BaseDomain with EtcdDiscoveryDomain
abhinavdahiya Jan 31, 2019
577f8ff
controller/template: use updated controllerconfig
abhinavdahiya Jan 31, 2019
b5d93f9
controller/kubelet-config: udpate tests to use update controllerconfig
abhinavdahiya Jan 31, 2019
79837d3
operator: use infra and network objects to create controllerconfig
abhinavdahiya Feb 1, 2019
936cc67
operator: update server daemonset to use new apiserver_url
abhinavdahiya Feb 1, 2019
a31438a
cmd: add new informers for infra and network
abhinavdahiya Feb 1, 2019
936edd2
vendor: bump openshift/{api,client-go}
abhinavdahiya Feb 1, 2019
6b08eaf
pkg/operator: add doc comments
abhinavdahiya Feb 13, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 13 additions & 4 deletions Gopkg.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 8 additions & 6 deletions cmd/common/controller_context.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package common
import (
"time"

configinformers "github.com/openshift/client-go/config/informers/externalversions"
mcfginformers "github.com/openshift/machine-config-operator/pkg/generated/informers/externalversions"
apiextinformers "k8s.io/apiextensions-apiserver/pkg/client/informers/externalversions"
"k8s.io/apimachinery/pkg/runtime/schema"
Expand All @@ -18,15 +19,14 @@ type ControllerContext struct {
KubeInformerFactory informers.SharedInformerFactory
KubeNamespacedInformerFactory informers.SharedInformerFactory
APIExtInformerFactory apiextinformers.SharedInformerFactory
ConfigInformerFactory configinformers.SharedInformerFactory

AvailableResources map[schema.GroupVersionResource]bool

Stop <-chan struct{}

InformersStarted chan struct{}

KubeInformersStarted chan struct{}

ResyncPeriod func() time.Duration
}

Expand All @@ -35,11 +35,13 @@ func CreateControllerContext(cb *ClientBuilder, stop <-chan struct{}, targetName
client := cb.MachineConfigClientOrDie("machine-config-shared-informer")
kubeClient := cb.KubeClientOrDie("kube-shared-informer")
apiExtClient := cb.APIExtClientOrDie("apiext-shared-informer")
configClient := cb.ConfigClientOrDie("config-shared-informer")
sharedInformers := mcfginformers.NewSharedInformerFactory(client, resyncPeriod()())
sharedNamespacedInformers := mcfginformers.NewFilteredSharedInformerFactory(client, resyncPeriod()(), targetNamespace, nil)
kubeSharedInformer := informers.NewSharedInformerFactory(kubeClient, resyncPeriod()())
kubeNamespacedSharedInformer := informers.NewFilteredSharedInformerFactory(kubeClient, resyncPeriod()(), targetNamespace, nil)
apiExtSharedInformer := apiextinformers.NewSharedInformerFactory(apiExtClient, resyncPeriod()())
configSharedInformer := configinformers.NewSharedInformerFactory(configClient, resyncPeriod()())

return &ControllerContext{
ClientBuilder: cb,
Expand All @@ -48,9 +50,9 @@ func CreateControllerContext(cb *ClientBuilder, stop <-chan struct{}, targetName
KubeInformerFactory: kubeSharedInformer,
KubeNamespacedInformerFactory: kubeNamespacedSharedInformer,
APIExtInformerFactory: apiExtSharedInformer,
Stop: stop,
InformersStarted: make(chan struct{}),
KubeInformersStarted: make(chan struct{}),
ResyncPeriod: resyncPeriod(),
ConfigInformerFactory: configSharedInformer,
Stop: stop,
InformersStarted: make(chan struct{}),
ResyncPeriod: resyncPeriod(),
}
}
1 change: 0 additions & 1 deletion cmd/machine-config-controller/start.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ func runStartCmd(cmd *cobra.Command, args []string) {
ctrlctx.InformerFactory.Start(ctrlctx.Stop)
ctrlctx.KubeInformerFactory.Start(ctrlctx.Stop)
close(ctrlctx.InformersStarted)
close(ctrlctx.KubeInformersStarted)

select {}
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/machine-config-daemon/start.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ func runStartCmd(cmd *cobra.Command, args []string) {
dn.EnterDegradedState(errors.Wrapf(err, "Checking initial state"))
}
ctx.KubeInformerFactory.Start(stopCh)
close(ctx.KubeInformersStarted)
close(ctx.InformersStarted)
}

glog.Info("Starting MachineConfigDaemon")
Expand Down
15 changes: 10 additions & 5 deletions cmd/machine-config-operator/bootstrap.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ var (
pullSecretFile string
configFile string
oscontentImage string
infraConfigFile string
networkConfigFile string
imagesConfigMapFile string
mccImage string
mcsImage string
Expand Down Expand Up @@ -60,6 +62,8 @@ func init() {
bootstrapCmd.MarkFlagRequired("setup-etcd-env-image")
bootstrapCmd.PersistentFlags().StringVar(&bootstrapOpts.configFile, "config-file", "", "ClusterConfig ConfigMap file.")
bootstrapCmd.MarkFlagRequired("config-file")
bootstrapCmd.PersistentFlags().StringVar(&bootstrapOpts.infraConfigFile, "infra-config-file", "/assets/manifests/cluster-infrastructure-02-config.yml", "File containing infrastructure.config.openshift.io manifest.")
bootstrapCmd.PersistentFlags().StringVar(&bootstrapOpts.networkConfigFile, "network-config-file", "/assets/manifests/cluster-network-02-config.yml", "File containing network.config.openshift.io manifest.")
}

func runBootstrapCmd(cmd *cobra.Command, args []string) {
Expand All @@ -71,15 +75,16 @@ func runBootstrapCmd(cmd *cobra.Command, args []string) {

imgs := operator.Images{
MachineConfigController: bootstrapOpts.mccImage,
MachineConfigDaemon: bootstrapOpts.mcdImage,
MachineConfigServer: bootstrapOpts.mcsImage,
MachineOSContent: bootstrapOpts.oscontentImage,
Etcd: bootstrapOpts.etcdImage,
SetupEtcdEnv: bootstrapOpts.setupEtcdEnvImage,
MachineConfigDaemon: bootstrapOpts.mcdImage,
MachineConfigServer: bootstrapOpts.mcsImage,
MachineOSContent: bootstrapOpts.oscontentImage,
Etcd: bootstrapOpts.etcdImage,
SetupEtcdEnv: bootstrapOpts.setupEtcdEnvImage,
}

if err := operator.RenderBootstrap(
bootstrapOpts.configFile,
bootstrapOpts.infraConfigFile, bootstrapOpts.networkConfigFile,
bootstrapOpts.etcdCAFile, bootstrapOpts.rootCAFile, bootstrapOpts.pullSecretFile,
imgs,
bootstrapOpts.destinationDir,
Expand Down
5 changes: 4 additions & 1 deletion cmd/machine-config-operator/start.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,8 @@ func runStartCmd(cmd *cobra.Command, args []string) {
ctrlctx.KubeInformerFactory.Start(ctrlctx.Stop)
ctrlctx.KubeNamespacedInformerFactory.Start(ctrlctx.Stop)
ctrlctx.APIExtInformerFactory.Start(ctrlctx.Stop)
close(ctrlctx.KubeInformersStarted)
ctrlctx.ConfigInformerFactory.Start(ctrlctx.Stop)
close(ctrlctx.InformersStarted)

select {}
}
Expand Down Expand Up @@ -93,6 +94,8 @@ func startControllers(ctx *common.ControllerContext) error {
ctx.KubeNamespacedInformerFactory.Rbac().V1().ClusterRoles(),
ctx.KubeNamespacedInformerFactory.Rbac().V1().ClusterRoleBindings(),
ctx.KubeNamespacedInformerFactory.Core().V1().ConfigMaps(),
ctx.ConfigInformerFactory.Config().V1().Infrastructures(),
ctx.ConfigInformerFactory.Config().V1().Networks(),
ctx.ClientBuilder.MachineConfigClientOrDie(componentName),
ctx.ClientBuilder.KubeClientOrDie(componentName),
ctx.ClientBuilder.APIExtClientOrDie(componentName),
Expand Down
3 changes: 1 addition & 2 deletions lib/resourcemerge/machineconfig.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,8 @@ func ensureMachineConfigSpec(modified *bool, existing *mcfgv1.MachineConfigSpec,
func ensureControllerConfigSpec(modified *bool, existing *mcfgv1.ControllerConfigSpec, required mcfgv1.ControllerConfigSpec) {
setStringIfSet(modified, &existing.ClusterDNSIP, required.ClusterDNSIP)
setStringIfSet(modified, &existing.CloudProviderConfig, required.CloudProviderConfig)
setStringIfSet(modified, &existing.ClusterName, required.ClusterName)
setStringIfSet(modified, &existing.Platform, required.Platform)
setStringIfSet(modified, &existing.BaseDomain, required.BaseDomain)
setStringIfSet(modified, &existing.EtcdDiscoveryDomain, required.EtcdDiscoveryDomain)
setStringIfSet(modified, &existing.SSHKey, required.SSHKey)
setStringIfSet(modified, &existing.OSImageURL, required.OSImageURL)

Expand Down
2 changes: 1 addition & 1 deletion manifests/machineconfigserver/daemonset.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ spec:
image: {{.Images.MachineConfigServer}}
args:
- "start"
- "--apiserver-url=https://{{.ControllerConfig.ClusterName}}-api.{{.ControllerConfig.BaseDomain}}:6443"
- "--apiserver-url={{.APIServerURL}}"
resources:
requests:
cpu: 20m
Expand Down
13 changes: 1 addition & 12 deletions pkg/apis/machineconfiguration.openshift.io/v1/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,16 +51,6 @@ type MCOConfig struct {

// MCOConfigSpec is the spec for MCOConfig resource.
type MCOConfigSpec struct {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this just an empty struct?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we don't need to duplicate here and controllerconfigspec.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The one place it was used was deleted too (operator/render.go) so can I ask to understand why we keep it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

k8s object typically have spec and status. So this needs to staty but, for now empty.

MCOConfig can be expanded later on to drive operator level configuration like
maybe placement of controller, server etc.
log levels for its operands and such...

Copy link
Member

@runcom runcom Feb 13, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Abhinav was faster than me at commenting so I didn't notice his comment before posting mine)

My take is this is still the MCO config object and we may add fields going forward (as well as deprecating and so on and so forth) so we stick it around but if we instead can remove it, let's nuke it

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

makes sense! @abhinavdahiya and @runcom (tag team!)

ClusterDNSIP string `json:"clusterDNSIP"`
CloudProviderConfig string `json:"cloudProviderConfig"`
ClusterName string `json:"clusterName"`

// The openshift platform, e.g. "libvirt", "openstack", "aws", or "none"
Platform string `json:"platform"`

BaseDomain string `json:"baseDomain"`

SSHKey string `json:"sshKey"`
}

// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
Expand Down Expand Up @@ -120,12 +110,11 @@ type ControllerConfig struct {
type ControllerConfigSpec struct {
ClusterDNSIP string `json:"clusterDNSIP"`
CloudProviderConfig string `json:"cloudProviderConfig"`
ClusterName string `json:"clusterName"`

// The openshift platform, e.g. "libvirt", "openstack", "aws", or "none"
Platform string `json:"platform"`

BaseDomain string `json:"baseDomain"`
EtcdDiscoveryDomain string `json:"etcdDiscoveryDomain"`

// CAs
EtcdCAData []byte `json:"etcdCAData"`
Expand Down
4 changes: 2 additions & 2 deletions pkg/controller/kubelet-config/kubelet_config_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package kubeletconfig

import (
"fmt"
"reflect"
"testing"
"time"
Expand Down Expand Up @@ -76,8 +77,7 @@ func newControllerConfig(name string) *mcfgv1.ControllerConfig {
TypeMeta: metav1.TypeMeta{APIVersion: mcfgv1.SchemeGroupVersion.String()},
ObjectMeta: metav1.ObjectMeta{Name: name, UID: types.UID(utilrand.String(5))},
Spec: mcfgv1.ControllerConfigSpec{
ClusterName: name,
BaseDomain: "tt.testing",
EtcdDiscoveryDomain: fmt.Sprintf("%s.tt.testing", name),
},
}
return cc
Expand Down
17 changes: 2 additions & 15 deletions pkg/controller/template/render.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,6 @@ func renderTemplate(config RenderConfig, path string, b []byte) ([]byte, error)
funcs["skip"] = skipMissing
funcs["etcdServerCertDNSNames"] = etcdServerCertDNSNames
funcs["etcdPeerCertDNSNames"] = etcdPeerCertDNSNames
funcs["apiServerURL"] = apiServerURL
funcs["cloudProvider"] = cloudProvider
tmpl, err := template.New(path).Funcs(funcs).Parse(string(b))
if err != nil {
Expand Down Expand Up @@ -330,10 +329,6 @@ func skipMissing(key string) (interface{}, error) {

// Process the {{etcdPeerCertDNSNames}} and {{etcdServerCertDNSNames}}
func etcdServerCertDNSNames(cfg RenderConfig) (interface{}, error) {
if cfg.BaseDomain == "" {
return nil, fmt.Errorf("invalid configuration")
}

var dnsNames = []string{
"localhost",
"etcd.kube-system.svc", // sign for the local etcd service name that cluster-network apiservers use to communicate
Expand All @@ -344,25 +339,17 @@ func etcdServerCertDNSNames(cfg RenderConfig) (interface{}, error) {
}

func etcdPeerCertDNSNames(cfg RenderConfig) (interface{}, error) {
if cfg.ClusterName == "" || cfg.BaseDomain == "" {
if cfg.EtcdDiscoveryDomain == "" {
return nil, fmt.Errorf("invalid configuration")
}

var dnsNames = []string{
"${ETCD_DNS_NAME}",
fmt.Sprintf("%s.%s", cfg.ClusterName, cfg.BaseDomain), // https://github.com/etcd-io/etcd/blob/583763261f1c843e07c1bf7fea5fb4cfb684fe87/Documentation/op-guide/clustering.md#dns-discovery
cfg.EtcdDiscoveryDomain, // https://github.com/etcd-io/etcd/blob/583763261f1c843e07c1bf7fea5fb4cfb684fe87/Documentation/op-guide/clustering.md#dns-discovery
}
return strings.Join(dnsNames, ","), nil
}

// generate apiserver url using cluster-name, basename
func apiServerURL(cfg RenderConfig) (interface{}, error) {
if cfg.ClusterName == "" || cfg.BaseDomain == "" {
return nil, fmt.Errorf("invalid configuration")
}
return fmt.Sprintf("https://%s-api.%s:6443", cfg.ClusterName, cfg.BaseDomain), nil
}

func cloudProvider(cfg RenderConfig) (interface{}, error) {
switch cfg.Platform {
case platformAWS:
Expand Down
Loading