Skip to content

Commit

Permalink
Set KFNBC as default (#460)
Browse files Browse the repository at this point in the history
* Set KFNBC as default

* update test

* address comments
  • Loading branch information
DaoDaoNoCode authored Sep 1, 2022
1 parent 7e0ad68 commit 9bed7af
Show file tree
Hide file tree
Showing 36 changed files with 334 additions and 215 deletions.
20 changes: 12 additions & 8 deletions backend/src/routes/api/cluster-settings/clusterSettingsUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import { KubeFastifyInstance, ClusterSettings } from '../../../types';
import { getDashboardConfig } from '../../../utils/resourceUtils';
import { V1ConfigMap } from '@kubernetes/client-node';
import { setDashboardConfig } from '../config/configUtils';
import { checkJupyterEnabled } from '../../../utils/componentUtils';

const jupyterhubCfg = 'jupyterhub-cfg';
const nbcCfg = 'notebook-controller-culler-config';
const segmentKeyCfg = 'odh-segment-key-config';
Expand All @@ -28,19 +30,20 @@ export const updateClusterSettings = async (
const namespace = fastify.kube.namespace;
const { pvcSize, cullerTimeout, userTrackingEnabled, notebookTolerationSettings } = request.body;
const dashConfig = getDashboardConfig();
const isJupyterEnabled = checkJupyterEnabled();
try {
await patchCM(fastify, segmentKeyCfg, {
data: { segmentKeyEnabled: String(userTrackingEnabled) },
}).catch((e) => {
fastify.log.error('Failed to update segment key enabled: ' + e.message);
});
if (pvcSize && cullerTimeout) {
if (dashConfig.spec?.notebookController?.enabled) {
if (isJupyterEnabled) {
await setDashboardConfig(fastify, {
spec: {
dashboardConfig: dashConfig.spec.dashboardConfig,
notebookController: {
enabled: dashConfig.spec.notebookController.enabled,
enabled: isJupyterEnabled,
pvcSize: `${pvcSize}Gi`,
notebookTolerationSettings: {
enabled: notebookTolerationSettings.enabled,
Expand Down Expand Up @@ -92,7 +95,7 @@ export const updateClusterSettings = async (
});
}
}
if (dashConfig.spec.notebookController?.enabled) {
if (isJupyterEnabled) {
await rolloutDeployment(fastify, namespace, 'notebook-controller-deployment');
} else {
const jupyterhubCM = await coreV1Api.readNamespacedConfigMap(jupyterhubCfg, namespace);
Expand Down Expand Up @@ -124,22 +127,23 @@ export const getClusterSettings = async (
...DEFAULT_CLUSTER_SETTINGS,
};
const dashConfig = getDashboardConfig();
const isJupyterEnabled = checkJupyterEnabled();
try {
const segmentEnabledRes = await coreV1Api.readNamespacedConfigMap(segmentKeyCfg, namespace);
clusterSettings.userTrackingEnabled = segmentEnabledRes.body.data.segmentKeyEnabled === 'true';
} catch (e) {
fastify.log.error('Error retrieving segment key enabled: ' + e.toString());
}
if (dashConfig.spec?.notebookController?.enabled) {
if (isJupyterEnabled) {
clusterSettings.pvcSize = DEFAULT_PVC_SIZE;
if (dashConfig.spec.notebookController.pvcSize) {
if (dashConfig.spec.notebookController?.pvcSize) {
clusterSettings.pvcSize = Number(
dashConfig.spec.notebookController.pvcSize.replace('Gi', ''),
dashConfig.spec.notebookController?.pvcSize.replace('Gi', ''),
);
}
if (dashConfig.spec.notebookController.notebookTolerationSettings) {
if (dashConfig.spec.notebookController?.notebookTolerationSettings) {
clusterSettings.notebookTolerationSettings =
dashConfig.spec.notebookController.notebookTolerationSettings;
dashConfig.spec.notebookController?.notebookTolerationSettings;
}
clusterSettings.cullerTimeout = DEFAULT_CULLER_TIMEOUT; // For backwards compatibility with jupyterhub and less changes to UI
await fastify.kube.coreV1Api
Expand Down
4 changes: 2 additions & 2 deletions backend/src/routes/api/components/list.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
import { FastifyRequest } from 'fastify';
import { KubeFastifyInstance, OdhApplication } from '../../../types';
import { getIsJupyterEnabled, getRouteForApplication } from '../../../utils/componentUtils';
import { checkJupyterEnabled, getRouteForApplication } from '../../../utils/componentUtils';
import { getApplicationDefs, updateApplicationDefs } from '../../../utils/resourceUtils';

export const listComponents = async (
fastify: KubeFastifyInstance,
request: FastifyRequest,
): Promise<OdhApplication[]> => {
const applicationDefs = getApplicationDefs().filter(
(component) => component.metadata.name !== (getIsJupyterEnabled() ? 'jupyterhub' : 'jupyter'),
(component) => component.metadata.name !== (checkJupyterEnabled() ? 'jupyterhub' : 'jupyter'),
);
const installedComponents = [];
const query = request.query as { [key: string]: string };
Expand Down
4 changes: 2 additions & 2 deletions backend/src/routes/api/quickstarts/quickStartUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import * as jsYaml from 'js-yaml';
import { yamlRegExp } from '../../../utils/constants';
import { KubeFastifyInstance, QuickStart } from '../../../types';
import { getComponentFeatureFlags } from '../../../utils/features';
import { getIsJupyterEnabled } from '../../../utils/componentUtils';
import { checkJupyterEnabled } from '../../../utils/componentUtils';

const quickStartsGroup = 'console.openshift.io';
const quickStartsVersion = 'apiextensions.k8s.io/v1';
Expand Down Expand Up @@ -34,7 +34,7 @@ export const getInstalledQuickStarts = async (
const doc: QuickStart = jsYaml.load(
fs.readFileSync(path.join(normalizedPath, file), 'utf8'),
);
if (doc.spec.appName === 'jupyterhub' && getIsJupyterEnabled()) {
if (doc.spec.appName === 'jupyterhub' && checkJupyterEnabled()) {
doc.spec.appName = 'jupyter';
}
if (!doc.spec.featureFlag || featureFlags[doc.spec.featureFlag]) {
Expand Down
4 changes: 2 additions & 2 deletions backend/src/utils/componentUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -284,5 +284,5 @@ export const convertLabelsToString = (labels: { [key: string]: string }): string
return outputString;
};

export const getIsJupyterEnabled = (): boolean =>
!!getDashboardConfig().spec.notebookController?.enabled;
export const checkJupyterEnabled = (): boolean =>
getDashboardConfig().spec.notebookController?.enabled !== false;
3 changes: 3 additions & 0 deletions backend/src/utils/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ export const blankDashboardCR: DashboardConfig = {
disableAppLauncher: false,
disableUserManagement: false,
},
notebookController: {
enabled: true,
},
groupsConfig: {
adminGroups: 'odh-admins',
allowedGroups: 'system:authenticated',
Expand Down
4 changes: 2 additions & 2 deletions backend/src/utils/resourceUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import {
} from './resourceWatcher';
import { getComponentFeatureFlags } from './features';
import { yamlRegExp, blankDashboardCR } from './constants';
import { getIsAppEnabled, getIsJupyterEnabled, getRouteForClusterId } from './componentUtils';
import { getIsAppEnabled, checkJupyterEnabled, getRouteForClusterId } from './componentUtils';

const dashboardConfigMapName = 'odh-dashboard-config';
const consoleLinksGroup = 'console.openshift.io';
Expand Down Expand Up @@ -223,7 +223,7 @@ const fetchDocs = async (fastify: KubeFastifyInstance): Promise<OdhDocument[]> =
const doc: OdhDocument = jsYaml.load(
fs.readFileSync(path.join(normalizedPath, file), 'utf8'),
);
if (doc.spec.appName === 'jupyterhub' && getIsJupyterEnabled()) {
if (doc.spec.appName === 'jupyterhub' && checkJupyterEnabled()) {
doc.spec.appName = 'jupyter';
}
if (doc.spec.featureFlag) {
Expand Down
12 changes: 12 additions & 0 deletions frontend/src/__tests__/EnabledApplications.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,18 @@ import { mockEnabledApplications } from '../../__mocks__/mockEnabledApplications
import { render, screen } from '@testing-library/react';
import '@testing-library/jest-dom';

const dashboardConfig = {
spec: {
dashboardConfig: {
disableISVBadges: false,
},
},
};

jest.mock('../app/AppContext.ts', () => ({
useAppContext: () => ({ dashboardConfig }),
}));

describe('EnabledApplications', () => {
it('should display a message when there are no enabled applications', () => {
render(
Expand Down
10 changes: 3 additions & 7 deletions frontend/src/__tests__/ExploreApplications.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ jest.mock('react', () => {
lazy: (factory) => factory(),
};
});
jest.mock('../app/AppContext.ts', () => ({
useAppContext: () => ({ dashboardConfig }),
}));
jest.mock('../utilities/useWatchComponents', () => ({
useWatchComponents: () => ({
loaded: true,
Expand All @@ -55,13 +58,6 @@ jest.mock('../utilities/router', () => ({
return;
},
}));
jest.mock('../utilities/useWatchDashboardConfig', () => ({
useWatchDashboardConfig: () => ({
dashboardConfig,
loaded: true,
loadError: null,
}),
}));

// scrollIntoView is not implemented in jsdom
window.HTMLElement.prototype.scrollIntoView = jest.fn();
Expand Down
26 changes: 15 additions & 11 deletions frontend/src/app/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,31 +5,32 @@ import '@patternfly/patternfly/patternfly-addons.css';
import { Alert, Bullseye, Page, PageSection, Spinner } from '@patternfly/react-core';
import { detectUser } from '../redux/actions/actions';
import { useDesktopWidth } from '../utilities/useDesktopWidth';
import { useTrackHistory } from '../utilities/useTrackHistory';
import { useSegmentTracking } from '../utilities/useSegmentTracking';
import Header from './Header';
import Routes from './Routes';
import NavSidebar from './NavSidebar';
import ToastNotifications from '../components/ToastNotifications';
import AppNotificationDrawer from './AppNotificationDrawer';
import { useWatchBuildStatus } from '../utilities/useWatchBuildStatus';
import AppContext from './AppContext';
import { AppContext } from './AppContext';
import { useApplicationSettings } from './useApplicationSettings';
import { useUser } from '../redux/selectors';
import TelemetrySetup from './TelemetrySetup';

import './App.scss';
import { useWatchDashboardConfig } from 'utilities/useWatchDashboardConfig';
import { useUser } from '../redux/selectors';

const App: React.FC = () => {
const isDeskTop = useDesktopWidth();
const [isNavOpen, setIsNavOpen] = React.useState(isDeskTop);
const [notificationsOpen, setNotificationsOpen] = React.useState(false);
const { username, userError } = useUser();
const dispatch = useDispatch();
useSegmentTracking();
useTrackHistory();

const buildStatuses = useWatchBuildStatus();
const { dashboardConfig } = useWatchDashboardConfig();
const {
dashboardConfig,
loaded: configLoaded,
loadError: fetchConfigError,
} = useApplicationSettings();

React.useEffect(() => {
dispatch(detectUser());
Expand All @@ -43,16 +44,18 @@ const App: React.FC = () => {
setIsNavOpen(!isNavOpen);
};

if (!username) {
if (!username || !configLoaded || !dashboardConfig) {
// We do not have the data yet for who they are, we can't show the app. If we allow anything
// to render right now, the username is going to be blank and we won't know permissions
if (userError) {
// If we don't get the config we cannot show the pages, either
if (userError || fetchConfigError) {
// We likely don't have a username still so just show the error
// or we likely meet something wrong when fetching the dashboard config, we also show the error
return (
<Page>
<PageSection>
<Alert variant="danger" isInline title="General loading error">
{userError.message}
{userError ? userError.message : fetchConfigError?.message}
</Alert>
</PageSection>
</Page>
Expand Down Expand Up @@ -86,6 +89,7 @@ const App: React.FC = () => {
>
<Routes />
<ToastNotifications />
<TelemetrySetup />
</Page>
</AppContext.Provider>
);
Expand Down
8 changes: 4 additions & 4 deletions frontend/src/app/AppContext.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import * as React from 'react';
import { blankDashboardCR } from '../utilities/useWatchDashboardConfig';
import { BuildStatus, DashboardConfig } from '../types';

type AppContextProps = {
Expand All @@ -15,9 +14,10 @@ const defaultAppContext: AppContextProps = {
setIsNavOpen: () => undefined,
onNavToggle: () => undefined,
buildStatuses: [],
dashboardConfig: blankDashboardCR,
// At runtime dashboardConfig is never null -- DO NOT DO THIS usually
dashboardConfig: null as unknown as DashboardConfig,
};

const AppContext = React.createContext(defaultAppContext);
export const AppContext = React.createContext(defaultAppContext);

export default AppContext;
export const useAppContext = (): AppContextProps => React.useContext(AppContext);
17 changes: 7 additions & 10 deletions frontend/src/app/AppLauncher.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import openshiftLogo from '../images/openshift.svg';
import { RootState } from '../redux/types';
import { useWatchConsoleLinks } from '../utilities/useWatchConsoleLinks';
import { ODH_PRODUCT_NAME } from '../utilities/const';
import { DashboardCommonConfig } from '../types';
import { useAppContext } from './AppContext';

type ApplicationAction = {
label: string;
Expand Down Expand Up @@ -68,17 +68,16 @@ const sectionSortValue = (section: Section): number => {
}
};

type AppLauncherProps = {
dashboardConfig: DashboardCommonConfig; // Changed to avoid creating new types.
};

const AppLauncher: React.FC<AppLauncherProps> = ({ dashboardConfig }) => {
const AppLauncher: React.FC = () => {
const [isOpen, setIsOpen] = React.useState<boolean>(false);
const [clusterID, clusterBranding] = useSelector((state: RootState) => [
state.appState.clusterID,
state.appState.clusterBranding,
]);
const { consoleLinks } = useWatchConsoleLinks();
const { dashboardConfig } = useAppContext();

const disableClusterManager = dashboardConfig.spec.dashboardConfig.disableClusterManager;

const applicationSections = React.useMemo<Section[]>(() => {
const applicationLinks = consoleLinks
Expand All @@ -90,9 +89,7 @@ const AppLauncher: React.FC<AppLauncherProps> = ({ dashboardConfig }) => {

const getODHApplications = (): Section[] => {
const osConsoleAction = getOpenShiftConsoleAction();
const ocmAction = dashboardConfig.disableClusterManager
? null
: getOCMAction(clusterID, clusterBranding);
const ocmAction = disableClusterManager ? null : getOCMAction(clusterID, clusterBranding);

if (!osConsoleAction && !ocmAction) {
return [];
Expand Down Expand Up @@ -128,7 +125,7 @@ const AppLauncher: React.FC<AppLauncherProps> = ({ dashboardConfig }) => {
sections.sort((a, b) => sectionSortValue(a) - sectionSortValue(b));

return sections;
}, [clusterBranding, clusterID, consoleLinks, dashboardConfig.disableClusterManager]);
}, [clusterBranding, clusterID, consoleLinks, disableClusterManager]);

const onToggle = () => {
setIsOpen((prev) => !prev);
Expand Down
4 changes: 2 additions & 2 deletions frontend/src/app/Header.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,15 @@ import React from 'react';
import { Brand, PageHeader } from '@patternfly/react-core';
import HeaderTools from './HeaderTools';
import { ODH_LOGO, ODH_PRODUCT_NAME } from '../utilities/const';
import AppContext from './AppContext';
import { useAppContext } from './AppContext';
import { Link } from 'react-router-dom';

type HeaderProps = {
onNotificationsClick: () => void;
};

const Header: React.FC<HeaderProps> = ({ onNotificationsClick }) => {
const { isNavOpen, onNavToggle } = React.useContext(AppContext);
const { isNavOpen, onNavToggle } = useAppContext();
return (
<PageHeader
logo={
Expand Down
10 changes: 4 additions & 6 deletions frontend/src/app/HeaderTools.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ import {
import { CaretDownIcon, ExternalLinkAltIcon, QuestionCircleIcon } from '@patternfly/react-icons';
import { COMMUNITY_LINK, DOC_LINK, SUPPORT_LINK } from '../utilities/const';
import { AppNotification, State } from '../redux/types';
import { useWatchDashboardConfig } from '../utilities/useWatchDashboardConfig';
import AppLauncher from './AppLauncher';
import { useAppContext } from './AppContext';

interface HeaderToolsProps {
onNotificationsClick: () => void;
Expand All @@ -27,7 +27,7 @@ const HeaderTools: React.FC<HeaderToolsProps> = ({ onNotificationsClick }) => {
(state) => state.appState.notifications,
);
const userName: string = useSelector<State, string>((state) => state.appState.user || '');
const { dashboardConfig } = useWatchDashboardConfig().dashboardConfig.spec;
const { dashboardConfig } = useAppContext();

const newNotifications = React.useMemo(() => {
return notifications.filter((notification) => !notification.read).length;
Expand Down Expand Up @@ -67,7 +67,7 @@ const HeaderTools: React.FC<HeaderToolsProps> = ({ onNotificationsClick }) => {
</DropdownItem>,
);
}
if (SUPPORT_LINK && !dashboardConfig.disableSupport) {
if (SUPPORT_LINK && !dashboardConfig.spec.dashboardConfig.disableSupport) {
helpMenuItems.push(
<DropdownItem
key="support"
Expand Down Expand Up @@ -101,9 +101,7 @@ const HeaderTools: React.FC<HeaderToolsProps> = ({ onNotificationsClick }) => {
return (
<PageHeaderTools>
<PageHeaderToolsGroup className="hidden-xs">
{!dashboardConfig.disableAppLauncher ? (
<AppLauncher dashboardConfig={dashboardConfig} />
) : null}
{!dashboardConfig.spec.dashboardConfig.disableAppLauncher ? <AppLauncher /> : null}
<PageHeaderToolsItem>
<NotificationBadge isRead count={newNotifications} onClick={onNotificationsClick} />
</PageHeaderToolsItem>
Expand Down
Loading

0 comments on commit 9bed7af

Please sign in to comment.