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.

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

Update install script to allow conf name changes #392

Merged

Conversation

weikinhuang
Copy link
Contributor

@weikinhuang weikinhuang commented Oct 11, 2017

Description

This will allow users to upgrade the calico cni config to a different name while at the same time will clean up the old config files. This change is useful in the case where we want to move to a .conflist extension to enable additional plugins such as portmap during a upgrade.

This mirrors changes in coreos/flannel-cni#5.

Todos

  • Documentation
  • Release note

Release Note

Optional environment variable `CNI_OLD_CONF_NAME` for `install-cni.sh` to clean up old configs when migrating to a different `CNI_CONF_NAME`.

@CLAassistant
Copy link

CLAassistant commented Oct 11, 2017

CLA assistant check
All committers have signed the CLA.

@@ -128,18 +128,23 @@ sed -i s~__ETCD_CA_CERT_FILE__~${CNI_CONF_ETCD_CA:-}~g $TMP_CONF
sed -i s~__ETCD_ENDPOINTS__~${ETCD_ENDPOINTS:-}~g $TMP_CONF
sed -i s~__LOG_LEVEL__~${LOG_LEVEL:-warn}~g $TMP_CONF

FILENAME=${CNI_CONF_NAME:-10-calico.conf}
CNI_CONF_NAME=${CNI_CONF_NAME:-10-calico.conf}
CNI_OLD_NAME=${CNI_OLD_NAME:-10-calico.conf}
Copy link
Member

Choose a reason for hiding this comment

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

One minor thing - I think it would be good to name the config option CNI_OLD_CONF_NAME to make it more clear it's referring to the old CNI configuration file name, not something else.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure thing!

@weikinhuang weikinhuang force-pushed the conf-conflist-upgrade branch from a461fbd to ec0e68c Compare October 11, 2017 19:03
@caseydavenport caseydavenport merged commit 161e5f1 into projectcalico:master Oct 11, 2017
@caseydavenport caseydavenport added this to the Calico v2.6.2 milestone Oct 11, 2017
@weikinhuang
Copy link
Contributor Author

Thanks!

@weikinhuang
Copy link
Contributor Author

will a release be cut for this? I see that this has a v2.6.2 milestone label.

@caseydavenport
Copy link
Member

@weikinhuang sorry for the long delay.

This exists now in Calico v3.0.0-beta1.

We also need to port this to v2.6.x, which is now looking like v2.6.4.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants